Re: [syzbot] WARNING in do_proc_bulk

From: Alan Stern
Date: Mon May 03 2021 - 20:47:06 EST


On Mon, May 03, 2021 at 12:24:28PM -0700, Andrew Morton wrote:
> On Mon, 3 May 2021 14:56:14 -0400 Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> > >
> > > do_proc_bulk() is asking kmalloc for more than MAX_ORDER bytes, in
> > >
> > > tbuf = kmalloc(len1, GFP_KERNEL);
> >
> > This doesn't seem to be a bug. do_proc_bulk is simply trying to
> > allocate a kernel buffer for data passed to/from userspace. If a user
> > wants too much space all at once, that's their problem.
> >
> > As far as I know, the kmalloc API doesn't require the caller to filter
> > out requests for more the MAX_ORDER bytes. Only to be prepared to
> > handle failures -- which do_proc_bulk is all set for.
> >
> > Am I wrong about this? Should we add __GFP_NOWARN to the gfp flags?
>
> Yes, if the oversized request is a can-happen and the resulting error is handled
> appropriately, __GFP_NOWARN is the way to go.

Okay, let's see how this does.

Alan Stern

#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d2b6f8a1

Index: usb-devel/drivers/usb/core/devio.c
===================================================================
--- usb-devel.orig/drivers/usb/core/devio.c
+++ usb-devel/drivers/usb/core/devio.c
@@ -1218,7 +1218,12 @@ static int do_proc_bulk(struct usb_dev_s
ret = usbfs_increase_memory_usage(len1 + sizeof(struct urb));
if (ret)
return ret;
- tbuf = kmalloc(len1, GFP_KERNEL);
+
+ /*
+ * len1 can be almost arbitrarily large. Don't WARN if it's
+ * too big, just fail the request.
+ */
+ tbuf = kmalloc(len1, GFP_KERNEL | __GFP_NOWARN);
if (!tbuf) {
ret = -ENOMEM;
goto done;