Messages in this thread | | | Date | Sat, 31 May 2008 04:12:42 +0300 | Subject | Re: [PATCH BUGFIX -v2 -rc4] Smack: Respect 'unlabeled' netlabel mode | From | "Ahmed S. Darwish" <> |
| |
On Fri, May 30, 2008 at 04:25:00PM -0700, Andrew Morton wrote: > On Sat, 31 May 2008 02:57:51 +0300 > "Ahmed S. Darwish" <darwish.07@gmail.com> wrote: > > > + mutex_lock(&smack_ambient_lock); > > + nlsp->domain = kstrdup(smack_net_ambient, GFP_ATOMIC); > > + mutex_unlock(&smack_ambient_lock); > > no no no no no. And no. > > GFP_ATOMIC is *unreliable*. Using it in a "security" feature is a bug > - if it fails, the feature isn't secure any more. > > Failing to check the kmalloc() return value might be a bug. > > If we _need_ GFP_ATOMIC here then taking a mutex in a cannot-sleep > context is a bug. > > The patch adds a kmalloc but doesn't add a kfree. Is it leaky? > > Finally, why is there a need to take a lock around a single store > instruction?
Possibly the worst three lines written ever. GFP_ATOMIC line was cut-and-paste forgetting to change it to GFP_KERNEL and the lock is already useless.
--
"Better to light a candle, than curse the darkness"
Ahmed S. Darwish Homepage: http://darwish.07.googlepages.com Blog: http://darwish-07.blogspot.com
| |