| Message ID | 20170630075614.ywv3y3tptor5ox7g@mwanda (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
On Fri, 2017-06-30 at 10:56 +0300, Dan Carpenter wrote: > We accidentally return success instead of -ENOMEM on this failure > path. > > Fixes: 409dcf31538a ("selinux: Add a cache for quicker retreival of > PKey SIDs") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> NAK, that's intentional. See the comment just above the code in question. > > diff --git a/security/selinux/ibpkey.c b/security/selinux/ibpkey.c > index e3614ee5f1c0..36e61f622b5a 100644 > --- a/security/selinux/ibpkey.c > +++ b/security/selinux/ibpkey.c > @@ -160,8 +160,10 @@ static int sel_ib_pkey_sid_slow(u64 > subnet_prefix, u16 pkey_num, u32 *sid) > * is valid, it just won't be added to the cache. > */ > new = kzalloc(sizeof(*new), GFP_ATOMIC); > - if (!new) > + if (!new) { > + ret = -ENOMEM; > goto out; > + } > > new->psec.subnet_prefix = subnet_prefix; > new->psec.pkey = pkey_num;
Stephen Smalley wrote: > On Fri, 2017-06-30 at 10:56 +0300, Dan Carpenter wrote: > > We accidentally return success instead of -ENOMEM on this failure > > path. > > > > Fixes: 409dcf31538a ("selinux: Add a cache for quicker retreival of > > PKey SIDs") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > NAK, that's intentional. See the comment just above the code in > question. If allocation failure is no problem, please consider using GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN instead of GFP_ATOMIC, for memory reserves is mainly targeted for OOM victims.
On Fri, Jun 30, 2017 at 9:10 AM, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > Stephen Smalley wrote: >> On Fri, 2017-06-30 at 10:56 +0300, Dan Carpenter wrote: >> > We accidentally return success instead of -ENOMEM on this failure >> > path. >> > >> > Fixes: 409dcf31538a ("selinux: Add a cache for quicker retreival of >> > PKey SIDs") >> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> >> >> NAK, that's intentional. See the comment just above the code in >> question. > > If allocation failure is no problem, please consider using > GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN instead of > GFP_ATOMIC, for memory reserves is mainly targeted for OOM victims. I have a todo item to try and consolidate some of the SELinux object cache code, this seems like something worth experimenting with when that happens.
diff --git a/security/selinux/ibpkey.c b/security/selinux/ibpkey.c index e3614ee5f1c0..36e61f622b5a 100644 --- a/security/selinux/ibpkey.c +++ b/security/selinux/ibpkey.c @@ -160,8 +160,10 @@ static int sel_ib_pkey_sid_slow(u64 subnet_prefix, u16 pkey_num, u32 *sid) * is valid, it just won't be added to the cache. */ new = kzalloc(sizeof(*new), GFP_ATOMIC); - if (!new) + if (!new) { + ret = -ENOMEM; goto out; + } new->psec.subnet_prefix = subnet_prefix; new->psec.pkey = pkey_num;
We accidentally return success instead of -ENOMEM on this failure path. Fixes: 409dcf31538a ("selinux: Add a cache for quicker retreival of PKey SIDs") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>