Skip to content

Commit 4c9014f

Browse files
torvaldstorvalds
committed
Merge tag 'nfs-for-3.8-2' of git://git.linux-nfs.org/projects/trondmy/linux-nfs
Pull NFS client bugfixes from Trond Myklebust: - Fix a permissions problem when opening NFSv4 files that only have the exec bit set. - Fix a couple of typos in pNFS (inverted logic), and the mount parsing (missing pointer dereference). - Work around a series of deadlock issues due to workqueues using struct work_struct pointer address comparisons in the re-entrancy tests. Ensure that we don't free struct work_struct prematurely if our work function involves waiting for completion of other work items (e.g. by calling rpc_shutdown_client). - Revert the part of commit 168e4b3 that is causing unnecessary warnings to be issued in the nfsd callback code. * tag 'nfs-for-3.8-2' of git://git.linux-nfs.org/projects/trondmy/linux-nfs: nfs: avoid dereferencing null pointer in initiate_bulk_draining SUNRPC: Partial revert of commit 168e4b3 NFS: Ensure that we free the rpc_task after read and write cleanups are done SUNRPC: Ensure that we free the rpc_task after cleanups are done nfs: fix null checking in nfs_get_option_str() pnfs: Increase the refcount when LAYOUTGET fails the first time NFS: Fix access to suid/sgid executables
2 parents 5ce2955 + ecf0eb9 commit 4c9014f

File tree

9 files changed

+61
-31
lines changed

9 files changed

+61
-31
lines changed

‎fs/nfs/callback_proc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ static u32 initiate_bulk_draining(struct nfs_client *clp,
206206

207207
list_for_each_entry(lo, &server->layouts, plh_layouts) {
208208
ino = igrab(lo->plh_inode);
209-
if (ino)
209+
if (!ino)
210210
continue;
211211
spin_lock(&ino->i_lock);
212212
/* Is this layout in the process of being freed? */

‎fs/nfs/dir.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2153,12 +2153,16 @@ static int nfs_open_permission_mask(int openflags)
21532153
{
21542154
int mask = 0;
21552155

2156-
if ((openflags & O_ACCMODE) != O_WRONLY)
2157-
mask |= MAY_READ;
2158-
if ((openflags & O_ACCMODE) != O_RDONLY)
2159-
mask |= MAY_WRITE;
2160-
if (openflags & __FMODE_EXEC)
2161-
mask |= MAY_EXEC;
2156+
if (openflags & __FMODE_EXEC) {
2157+
/* ONLY check exec rights */
2158+
mask = MAY_EXEC;
2159+
} else {
2160+
if ((openflags & O_ACCMODE) != O_WRONLY)
2161+
mask |= MAY_READ;
2162+
if ((openflags & O_ACCMODE) != O_RDONLY)
2163+
mask |= MAY_WRITE;
2164+
}
2165+
21622166
return mask;
21632167
}
21642168

‎fs/nfs/nfs4proc.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,7 +1626,8 @@ static int _nfs4_recover_proc_open(struct nfs4_opendata *data)
16261626

16271627
static int nfs4_opendata_access(struct rpc_cred *cred,
16281628
struct nfs4_opendata *opendata,
1629-
struct nfs4_state *state, fmode_t fmode)
1629+
struct nfs4_state *state, fmode_t fmode,
1630+
int openflags)
16301631
{
16311632
struct nfs_access_entry cache;
16321633
u32 mask;
@@ -1638,11 +1639,14 @@ static int nfs4_opendata_access(struct rpc_cred *cred,
16381639

16391640
mask = 0;
16401641
/* don't check MAY_WRITE - a newly created file may not have
1641-
* write mode bits, but POSIX allows the creating process to write */
1642-
if (fmode & FMODE_READ)
1643-
mask |= MAY_READ;
1644-
if (fmode & FMODE_EXEC)
1645-
mask |= MAY_EXEC;
1642+
* write mode bits, but POSIX allows the creating process to write.
1643+
* use openflags to check for exec, because fmode won't
1644+
* always have FMODE_EXEC set when file open for exec. */
1645+
if (openflags & __FMODE_EXEC) {
1646+
/* ONLY check for exec rights */
1647+
mask = MAY_EXEC;
1648+
} else if (fmode & FMODE_READ)
1649+
mask = MAY_READ;
16461650

16471651
cache.cred = cred;
16481652
cache.jiffies = jiffies;
@@ -1896,7 +1900,7 @@ static int _nfs4_do_open(struct inode *dir,
18961900
if (server->caps & NFS_CAP_POSIX_LOCK)
18971901
set_bit(NFS_STATE_POSIX_LOCKS, &state->flags);
18981902

1899-
status = nfs4_opendata_access(cred, opendata, state, fmode);
1903+
status = nfs4_opendata_access(cred, opendata, state, fmode, flags);
19001904
if (status != 0)
19011905
goto err_opendata_put;
19021906

‎fs/nfs/pnfs.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ static void
254254
pnfs_layout_set_fail_bit(struct pnfs_layout_hdr *lo, int fail_bit)
255255
{
256256
lo->plh_retry_timestamp = jiffies;
257-
if (test_and_set_bit(fail_bit, &lo->plh_flags))
257+
if (!test_and_set_bit(fail_bit, &lo->plh_flags))
258258
atomic_inc(&lo->plh_refcount);
259259
}
260260

‎fs/nfs/read.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,16 @@ void nfs_readdata_release(struct nfs_read_data *rdata)
9191
put_nfs_open_context(rdata->args.context);
9292
if (rdata->pages.pagevec != rdata->pages.page_array)
9393
kfree(rdata->pages.pagevec);
94-
if (rdata != &read_header->rpc_data)
95-
kfree(rdata);
96-
else
94+
if (rdata == &read_header->rpc_data) {
9795
rdata->header = NULL;
96+
rdata = NULL;
97+
}
9898
if (atomic_dec_and_test(&hdr->refcnt))
9999
hdr->completion_ops->completion(hdr);
100+
/* Note: we only free the rpc_task after callbacks are done.
101+
* See the comment in rpc_free_task() for why
102+
*/
103+
kfree(rdata);
100104
}
101105
EXPORT_SYMBOL_GPL(nfs_readdata_release);
102106

‎fs/nfs/super.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ static int nfs_get_option_str(substring_t args[], char **option)
11521152
{
11531153
kfree(*option);
11541154
*option = match_strdup(args);
1155-
return !option;
1155+
return !*option;
11561156
}
11571157

11581158
static int nfs_get_option_ul(substring_t args[], unsigned long *option)

‎fs/nfs/write.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,16 @@ void nfs_writedata_release(struct nfs_write_data *wdata)
126126
put_nfs_open_context(wdata->args.context);
127127
if (wdata->pages.pagevec != wdata->pages.page_array)
128128
kfree(wdata->pages.pagevec);
129-
if (wdata != &write_header->rpc_data)
130-
kfree(wdata);
131-
else
129+
if (wdata == &write_header->rpc_data) {
132130
wdata->header = NULL;
131+
wdata = NULL;
132+
}
133133
if (atomic_dec_and_test(&hdr->refcnt))
134134
hdr->completion_ops->completion(hdr);
135+
/* Note: we only free the rpc_task after callbacks are done.
136+
* See the comment in rpc_free_task() for why
137+
*/
138+
kfree(wdata);
135139
}
136140
EXPORT_SYMBOL_GPL(nfs_writedata_release);
137141

‎net/sunrpc/clnt.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -610,11 +610,6 @@ EXPORT_SYMBOL_GPL(rpc_killall_tasks);
610610
*/
611611
void rpc_shutdown_client(struct rpc_clnt *clnt)
612612
{
613-
/*
614-
* To avoid deadlock, never call rpc_shutdown_client from a
615-
* workqueue context!
616-
*/
617-
WARN_ON_ONCE(current->flags & PF_WQ_WORKER);
618613
might_sleep();
619614

620615
dprintk_rcu("RPC: shutting down %s client for %s\n",

‎net/sunrpc/sched.c

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -934,16 +934,35 @@ struct rpc_task *rpc_new_task(const struct rpc_task_setup *setup_data)
934934
return task;
935935
}
936936

937+
/*
938+
* rpc_free_task - release rpc task and perform cleanups
939+
*
940+
* Note that we free up the rpc_task _after_ rpc_release_calldata()
941+
* in order to work around a workqueue dependency issue.
942+
*
943+
* Tejun Heo states:
944+
* "Workqueue currently considers two work items to be the same if they're
945+
* on the same address and won't execute them concurrently - ie. it
946+
* makes a work item which is queued again while being executed wait
947+
* for the previous execution to complete.
948+
*
949+
* If a work function frees the work item, and then waits for an event
950+
* which should be performed by another work item and *that* work item
951+
* recycles the freed work item, it can create a false dependency loop.
952+
* There really is no reliable way to detect this short of verifying
953+
* every memory free."
954+
*
955+
*/
937956
static void rpc_free_task(struct rpc_task *task)
938957
{
939-
const struct rpc_call_ops *tk_ops = task->tk_ops;
940-
void *calldata = task->tk_calldata;
958+
unsigned short tk_flags = task->tk_flags;
959+
960+
rpc_release_calldata(task->tk_ops, task->tk_calldata);
941961

942-
if (task->tk_flags & RPC_TASK_DYNAMIC) {
962+
if (tk_flags & RPC_TASK_DYNAMIC) {
943963
dprintk("RPC: %5u freeing task\n", task->tk_pid);
944964
mempool_free(task, rpc_task_mempool);
945965
}
946-
rpc_release_calldata(tk_ops, calldata);
947966
}
948967

949968
static void rpc_async_release(struct work_struct *work)

0 commit comments

Comments
 (0)