* [PATCH] nfsd: prevent NULL ptr derefs on fault injection @ 2012-11-27 16:31 Sasha Levin 2012-11-27 17:01 ` J. Bruce Fields 0 siblings, 1 reply; 4+ messages in thread From: Sasha Levin @ 2012-11-27 16:31 UTC (permalink / raw) To: bfields; +Cc: linux-nfs, linux-kernel, Sasha Levin A recent patch series has moved hashtable initialization to when the net struct is initialized. When injecting faults, we tried accessing the hashtables even if the struct wasn't really initialized (nfsd wasn't in use) - this caused a NULL ptr deref. A simple test would be: echo 1 > /sys/kernel/debug/nfsd/forget_locks Signed-off-by: Sasha Levin <sasha.levin@oracle.com> --- fs/nfsd/netns.h | 3 +++ fs/nfsd/nfs4state.c | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index 227b93e..c5806a57 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -83,5 +83,8 @@ struct nfsd_net { struct delayed_work laundromat_work; }; +/* Simple check to find out if a given net was properly initialized */ +#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl) + extern int nfsd_net_id; #endif /* __NFSD_NETNS_H__ */ diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index e75872f..0e7428c 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4598,6 +4598,9 @@ void nfsd_forget_clients(u64 num) int count = 0; struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); + if (!nfsd_netns_ready(nn)) + return; + nfs4_lock_state(); list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { expire_client(clp); @@ -4643,6 +4646,9 @@ void nfsd_forget_locks(u64 num) int count; struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); + if (!nfsd_netns_ready(nn)) + return; + nfs4_lock_state(); count = nfsd_release_n_owners(num, false, release_lockowner_sop, nn); nfs4_unlock_state(); @@ -4655,6 +4661,9 @@ void nfsd_forget_openowners(u64 num) int count; struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); + if (!nfsd_netns_ready(nn)) + return; + nfs4_lock_state(); count = nfsd_release_n_owners(num, true, release_openowner_sop, nn); nfs4_unlock_state(); -- 1.8.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: prevent NULL ptr derefs on fault injection 2012-11-27 16:31 [PATCH] nfsd: prevent NULL ptr derefs on fault injection Sasha Levin @ 2012-11-27 17:01 ` J. Bruce Fields 2012-11-27 17:06 ` Bryan Schumaker 0 siblings, 1 reply; 4+ messages in thread From: J. Bruce Fields @ 2012-11-27 17:01 UTC (permalink / raw) To: Sasha Levin; +Cc: linux-nfs, linux-kernel, bjschuma On Tue, Nov 27, 2012 at 11:31:11AM -0500, Sasha Levin wrote: > A recent patch series has moved hashtable initialization to when the net > struct is initialized. > > When injecting faults, we tried accessing the hashtables even if the struct > wasn't really initialized (nfsd wasn't in use) - this caused a NULL ptr > deref. Thanks, adding Bryan to cc.--b. > > A simple test would be: > > echo 1 > /sys/kernel/debug/nfsd/forget_locks > > Signed-off-by: Sasha Levin <sasha.levin@oracle.com> > --- > fs/nfsd/netns.h | 3 +++ > fs/nfsd/nfs4state.c | 9 +++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h > index 227b93e..c5806a57 100644 > --- a/fs/nfsd/netns.h > +++ b/fs/nfsd/netns.h > @@ -83,5 +83,8 @@ struct nfsd_net { > struct delayed_work laundromat_work; > }; > > +/* Simple check to find out if a given net was properly initialized */ > +#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl) > + > extern int nfsd_net_id; > #endif /* __NFSD_NETNS_H__ */ > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index e75872f..0e7428c 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -4598,6 +4598,9 @@ void nfsd_forget_clients(u64 num) > int count = 0; > struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); > > + if (!nfsd_netns_ready(nn)) > + return; > + > nfs4_lock_state(); > list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { > expire_client(clp); > @@ -4643,6 +4646,9 @@ void nfsd_forget_locks(u64 num) > int count; > struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); > > + if (!nfsd_netns_ready(nn)) > + return; > + > nfs4_lock_state(); > count = nfsd_release_n_owners(num, false, release_lockowner_sop, nn); > nfs4_unlock_state(); > @@ -4655,6 +4661,9 @@ void nfsd_forget_openowners(u64 num) > int count; > struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); > > + if (!nfsd_netns_ready(nn)) > + return; > + > nfs4_lock_state(); > count = nfsd_release_n_owners(num, true, release_openowner_sop, nn); > nfs4_unlock_state(); > -- > 1.8.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: prevent NULL ptr derefs on fault injection 2012-11-27 17:01 ` J. Bruce Fields @ 2012-11-27 17:06 ` Bryan Schumaker 2012-11-27 17:37 ` Bryan Schumaker 0 siblings, 1 reply; 4+ messages in thread From: Bryan Schumaker @ 2012-11-27 17:06 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Sasha Levin, linux-nfs, linux-kernel On 11/27/2012 12:01 PM, J. Bruce Fields wrote: > On Tue, Nov 27, 2012 at 11:31:11AM -0500, Sasha Levin wrote: >> A recent patch series has moved hashtable initialization to when the net >> struct is initialized. >> >> When injecting faults, we tried accessing the hashtables even if the struct >> wasn't really initialized (nfsd wasn't in use) - this caused a NULL ptr >> deref. > > Thanks, adding Bryan to cc.--b. I was just looking over this :). Unfortunately, this patch changes code that my most recent patch set removes so my patches will need to change again if this goes in first. I'm looking for the best place to put this check to avoid having to change each of the forget_something() functions. - Bryan > >> >> A simple test would be: >> >> echo 1 > /sys/kernel/debug/nfsd/forget_locks >> >> Signed-off-by: Sasha Levin <sasha.levin@oracle.com> >> --- >> fs/nfsd/netns.h | 3 +++ >> fs/nfsd/nfs4state.c | 9 +++++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h >> index 227b93e..c5806a57 100644 >> --- a/fs/nfsd/netns.h >> +++ b/fs/nfsd/netns.h >> @@ -83,5 +83,8 @@ struct nfsd_net { >> struct delayed_work laundromat_work; >> }; >> >> +/* Simple check to find out if a given net was properly initialized */ >> +#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl) >> + >> extern int nfsd_net_id; >> #endif /* __NFSD_NETNS_H__ */ >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index e75872f..0e7428c 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -4598,6 +4598,9 @@ void nfsd_forget_clients(u64 num) >> int count = 0; >> struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); >> >> + if (!nfsd_netns_ready(nn)) >> + return; >> + >> nfs4_lock_state(); >> list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { >> expire_client(clp); >> @@ -4643,6 +4646,9 @@ void nfsd_forget_locks(u64 num) >> int count; >> struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); >> >> + if (!nfsd_netns_ready(nn)) >> + return; >> + >> nfs4_lock_state(); >> count = nfsd_release_n_owners(num, false, release_lockowner_sop, nn); >> nfs4_unlock_state(); >> @@ -4655,6 +4661,9 @@ void nfsd_forget_openowners(u64 num) >> int count; >> struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); >> >> + if (!nfsd_netns_ready(nn)) >> + return; >> + >> nfs4_lock_state(); >> count = nfsd_release_n_owners(num, true, release_openowner_sop, nn); >> nfs4_unlock_state(); >> -- >> 1.8.0 >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] nfsd: prevent NULL ptr derefs on fault injection 2012-11-27 17:06 ` Bryan Schumaker @ 2012-11-27 17:37 ` Bryan Schumaker 0 siblings, 0 replies; 4+ messages in thread From: Bryan Schumaker @ 2012-11-27 17:37 UTC (permalink / raw) To: J. Bruce Fields; +Cc: Sasha Levin, linux-nfs, linux-kernel On 11/27/2012 12:06 PM, Bryan Schumaker wrote: > On 11/27/2012 12:01 PM, J. Bruce Fields wrote: >> On Tue, Nov 27, 2012 at 11:31:11AM -0500, Sasha Levin wrote: >>> A recent patch series has moved hashtable initialization to when the net >>> struct is initialized. >>> >>> When injecting faults, we tried accessing the hashtables even if the struct >>> wasn't really initialized (nfsd wasn't in use) - this caused a NULL ptr >>> deref. >> >> Thanks, adding Bryan to cc.--b. > > I was just looking over this :). Unfortunately, this patch changes code that my most recent patch set removes so my patches will need to change again if this goes in first. I'm looking for the best place to put this check to avoid having to change each of the forget_something() functions. > > - Bryan This works on top of my most recent patches: diff --git a/fs/nfsd/fault_inject.c b/fs/nfsd/fault_inject.c index 699de07..235162a 100644 --- a/fs/nfsd/fault_inject.c +++ b/fs/nfsd/fault_inject.c @@ -76,6 +76,9 @@ static void nfsd_inject_set_client(struct nfsd_fault_inject_op *op, u64 count; struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); + if (!nfsd_netns_ready(nn)) + return; + nfs4_lock_state(); clp = nfsd_find_client(nn, addr, addr_size); if (clp) { diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h index 227b93e..c5806a57 100644 --- a/fs/nfsd/netns.h +++ b/fs/nfsd/netns.h @@ -83,5 +83,8 @@ struct nfsd_net { struct delayed_work laundromat_work; }; +/* Simple check to find out if a given net was properly initialized */ +#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl) + extern int nfsd_net_id; #endif /* __NFSD_NETNS_H__ */ diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index b32414e..84295d9 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -4739,6 +4739,9 @@ u64 nfsd_for_n_state(u64 max, u64 (*func)(struct nfsd_net *, struct nfs4_client u64 count = 0; struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); + if (!nfsd_netns_ready(nn)) + return 0; + list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { count += func(nn, clp, max - count); if ((max != 0) && (count >= max)) >> >>> >>> A simple test would be: >>> >>> echo 1 > /sys/kernel/debug/nfsd/forget_locks >>> >>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com> >>> --- >>> fs/nfsd/netns.h | 3 +++ >>> fs/nfsd/nfs4state.c | 9 +++++++++ >>> 2 files changed, 12 insertions(+) >>> >>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h >>> index 227b93e..c5806a57 100644 >>> --- a/fs/nfsd/netns.h >>> +++ b/fs/nfsd/netns.h >>> @@ -83,5 +83,8 @@ struct nfsd_net { >>> struct delayed_work laundromat_work; >>> }; >>> >>> +/* Simple check to find out if a given net was properly initialized */ >>> +#define nfsd_netns_ready(nn) ((nn)->sessionid_hashtbl) >>> + >>> extern int nfsd_net_id; >>> #endif /* __NFSD_NETNS_H__ */ >>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >>> index e75872f..0e7428c 100644 >>> --- a/fs/nfsd/nfs4state.c >>> +++ b/fs/nfsd/nfs4state.c >>> @@ -4598,6 +4598,9 @@ void nfsd_forget_clients(u64 num) >>> int count = 0; >>> struct nfsd_net *nn = net_generic(current->nsproxy->net_ns, nfsd_net_id); >>> >>> + if (!nfsd_netns_ready(nn)) >>> + return; >>> + >>> nfs4_lock_state(); >>> list_for_each_entry_safe(clp, next, &nn->client_lru, cl_lru) { >>> expire_client(clp); >>> @@ -4643,6 +4646,9 @@ void nfsd_forget_locks(u64 num) >>> int count; >>> struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); >>> >>> + if (!nfsd_netns_ready(nn)) >>> + return; >>> + >>> nfs4_lock_state(); >>> count = nfsd_release_n_owners(num, false, release_lockowner_sop, nn); >>> nfs4_unlock_state(); >>> @@ -4655,6 +4661,9 @@ void nfsd_forget_openowners(u64 num) >>> int count; >>> struct nfsd_net *nn = net_generic(&init_net, nfsd_net_id); >>> >>> + if (!nfsd_netns_ready(nn)) >>> + return; >>> + >>> nfs4_lock_state(); >>> count = nfsd_release_n_owners(num, true, release_openowner_sop, nn); >>> nfs4_unlock_state(); >>> -- >>> 1.8.0 >>> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-11-27 17:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-27 16:31 [PATCH] nfsd: prevent NULL ptr derefs on fault injection Sasha Levin 2012-11-27 17:01 ` J. Bruce Fields 2012-11-27 17:06 ` Bryan Schumaker 2012-11-27 17:37 ` Bryan Schumaker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).