Messages in this thread | | | Date | Fri, 6 Mar 2009 12:03:13 +0300 | From | Alexey Dobriyan <> | Subject | Re: [RFC][PATCH] kmemdup_from_user(): introduce |
| |
On Fri, Mar 06, 2009 at 04:27:53PM +0800, Li Zefan wrote: > Alexey Dobriyan wrote: > > On Fri, Mar 06, 2009 at 03:04:12PM +0800, Li Zefan wrote: > >> I notice there are many places doing copy_from_user() which follows > >> kmalloc(): > >> > >> dst = kmalloc(len, GFP_KERNEL); > >> if (!dst) > >> return -ENOMEM; > >> if (copy_from_user(dst, src, len)) { > >> kfree(dst); > >> return -EFAULT > >> } > >> > >> kmemdup_from_user() is a wrapper of the above code. With this new > >> function, we don't have to write 'len' twice, which can lead to > >> typos/mistakes. It also produces smaller code. > > > > Name totally sucks, it mixes kernel idiom of allocation with purely > > userspace function. > > > > I'm not good at English, and I don't know why "kernel memory duplicated > from user space" is so bad... > > or memdup_user() ? > > >> A qucik grep shows 250+ places where kmemdup_from_user() *may* be > >> used. I'll prepare a patchset to do this conversion. > > > > 250? > > > > I just found out how many copy_from_user() following km/zalloc(), so > not all of them are replace-able. > > > Let's not add wrapper for every two lines that happen to be used > > together. > > > > Why not if we have good reasons? And I don't think we can call this > "happen to" if there are 250+ of them?
Please, read through them. This "250+" number suddenly will become like 20, because wrapper is not good enough.
| |