From: Christoph Hellwig <hch@lst.de>
To: kbuild-all@lists.01.org
Subject: Re: drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from_user() 'tbuf' too small (4096 vs 8192)
Date: Mon, 10 Aug 2020 14:12:23 +0200 [thread overview]
Message-ID: <20200810121222.GA18639@lst.de> (raw)
In-Reply-To: <202008081337.Z6BAxT0d%lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]
As far as I can tell the warning is valid as we copy a user controlled
amount into a fixed sized buffer. But this an old condition not actually
created by this commit..
On Sat, Aug 08, 2020 at 01:26:39PM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 049eb096da48db0421dd5e358b9b082a1a8a2025
> commit: c17536d0abde2fd24afca542e3bb73b45a299633 usb: usbfs: stop using compat_alloc_user_space
> date: 2 weeks ago
> config: nds32-randconfig-m031-20200808 (attached as .config)
> compiler: nds32le-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from_user() 'tbuf' too small (4096 vs 8192)
>
> vim +/tbuf +1155 drivers/usb/core/devio.c
>
> 1104
> 1105 static int do_proc_control(struct usb_dev_state *ps,
> 1106 struct usbdevfs_ctrltransfer *ctrl)
> 1107 {
> 1108 struct usb_device *dev = ps->dev;
> 1109 unsigned int tmo;
> 1110 unsigned char *tbuf;
> 1111 unsigned wLength;
> 1112 int i, pipe, ret;
> 1113
> 1114 ret = check_ctrlrecip(ps, ctrl->bRequestType, ctrl->bRequest,
> 1115 ctrl->wIndex);
> 1116 if (ret)
> 1117 return ret;
> 1118 wLength = ctrl->wLength; /* To suppress 64k PAGE_SIZE warning */
> 1119 if (wLength > PAGE_SIZE)
> 1120 return -EINVAL;
> 1121 ret = usbfs_increase_memory_usage(PAGE_SIZE + sizeof(struct urb) +
> 1122 sizeof(struct usb_ctrlrequest));
> 1123 if (ret)
> 1124 return ret;
> 1125 tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
> 1126 if (!tbuf) {
> 1127 ret = -ENOMEM;
> 1128 goto done;
> 1129 }
> 1130 tmo = ctrl->timeout;
> 1131 snoop(&dev->dev, "control urb: bRequestType=%02x "
> 1132 "bRequest=%02x wValue=%04x "
> 1133 "wIndex=%04x wLength=%04x\n",
> 1134 ctrl->bRequestType, ctrl->bRequest, ctrl->wValue,
> 1135 ctrl->wIndex, ctrl->wLength);
> 1136 if (ctrl->bRequestType & 0x80) {
> 1137 pipe = usb_rcvctrlpipe(dev, 0);
> 1138 snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0);
> 1139
> 1140 usb_unlock_device(dev);
> 1141 i = usb_control_msg(dev, pipe, ctrl->bRequest,
> 1142 ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
> 1143 tbuf, ctrl->wLength, tmo);
> 1144 usb_lock_device(dev);
> 1145 snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE,
> 1146 tbuf, max(i, 0));
> 1147 if ((i > 0) && ctrl->wLength) {
> 1148 if (copy_to_user(ctrl->data, tbuf, i)) {
> 1149 ret = -EFAULT;
> 1150 goto done;
> 1151 }
> 1152 }
> 1153 } else {
> 1154 if (ctrl->wLength) {
> > 1155 if (copy_from_user(tbuf, ctrl->data, ctrl->wLength)) {
> 1156 ret = -EFAULT;
> 1157 goto done;
> 1158 }
> 1159 }
> 1160 pipe = usb_sndctrlpipe(dev, 0);
> 1161 snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT,
> 1162 tbuf, ctrl->wLength);
> 1163
> 1164 usb_unlock_device(dev);
> 1165 i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl->bRequest,
> 1166 ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
> 1167 tbuf, ctrl->wLength, tmo);
> 1168 usb_lock_device(dev);
> 1169 snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0);
> 1170 }
> 1171 if (i < 0 && i != -EPIPE) {
> 1172 dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
> 1173 "failed cmd %s rqt %u rq %u len %u ret %d\n",
> 1174 current->comm, ctrl->bRequestType, ctrl->bRequest,
> 1175 ctrl->wLength, i);
> 1176 }
> 1177 ret = i;
> 1178 done:
> 1179 free_page((unsigned long) tbuf);
> 1180 usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) +
> 1181 sizeof(struct usb_ctrlrequest));
> 1182 return ret;
> 1183 }
> 1184
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
---end quoted text---
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: kernel test robot <lkp@intel.com>
Cc: Christoph Hellwig <hch@lst.de>,
kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from_user() 'tbuf' too small (4096 vs 8192)
Date: Mon, 10 Aug 2020 14:12:23 +0200 [thread overview]
Message-ID: <20200810121222.GA18639@lst.de> (raw)
In-Reply-To: <202008081337.Z6BAxT0d%lkp@intel.com>
As far as I can tell the warning is valid as we copy a user controlled
amount into a fixed sized buffer. But this an old condition not actually
created by this commit..
On Sat, Aug 08, 2020 at 01:26:39PM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 049eb096da48db0421dd5e358b9b082a1a8a2025
> commit: c17536d0abde2fd24afca542e3bb73b45a299633 usb: usbfs: stop using compat_alloc_user_space
> date: 2 weeks ago
> config: nds32-randconfig-m031-20200808 (attached as .config)
> compiler: nds32le-linux-gcc (GCC) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from_user() 'tbuf' too small (4096 vs 8192)
>
> vim +/tbuf +1155 drivers/usb/core/devio.c
>
> 1104
> 1105 static int do_proc_control(struct usb_dev_state *ps,
> 1106 struct usbdevfs_ctrltransfer *ctrl)
> 1107 {
> 1108 struct usb_device *dev = ps->dev;
> 1109 unsigned int tmo;
> 1110 unsigned char *tbuf;
> 1111 unsigned wLength;
> 1112 int i, pipe, ret;
> 1113
> 1114 ret = check_ctrlrecip(ps, ctrl->bRequestType, ctrl->bRequest,
> 1115 ctrl->wIndex);
> 1116 if (ret)
> 1117 return ret;
> 1118 wLength = ctrl->wLength; /* To suppress 64k PAGE_SIZE warning */
> 1119 if (wLength > PAGE_SIZE)
> 1120 return -EINVAL;
> 1121 ret = usbfs_increase_memory_usage(PAGE_SIZE + sizeof(struct urb) +
> 1122 sizeof(struct usb_ctrlrequest));
> 1123 if (ret)
> 1124 return ret;
> 1125 tbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
> 1126 if (!tbuf) {
> 1127 ret = -ENOMEM;
> 1128 goto done;
> 1129 }
> 1130 tmo = ctrl->timeout;
> 1131 snoop(&dev->dev, "control urb: bRequestType=%02x "
> 1132 "bRequest=%02x wValue=%04x "
> 1133 "wIndex=%04x wLength=%04x\n",
> 1134 ctrl->bRequestType, ctrl->bRequest, ctrl->wValue,
> 1135 ctrl->wIndex, ctrl->wLength);
> 1136 if (ctrl->bRequestType & 0x80) {
> 1137 pipe = usb_rcvctrlpipe(dev, 0);
> 1138 snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT, NULL, 0);
> 1139
> 1140 usb_unlock_device(dev);
> 1141 i = usb_control_msg(dev, pipe, ctrl->bRequest,
> 1142 ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
> 1143 tbuf, ctrl->wLength, tmo);
> 1144 usb_lock_device(dev);
> 1145 snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE,
> 1146 tbuf, max(i, 0));
> 1147 if ((i > 0) && ctrl->wLength) {
> 1148 if (copy_to_user(ctrl->data, tbuf, i)) {
> 1149 ret = -EFAULT;
> 1150 goto done;
> 1151 }
> 1152 }
> 1153 } else {
> 1154 if (ctrl->wLength) {
> > 1155 if (copy_from_user(tbuf, ctrl->data, ctrl->wLength)) {
> 1156 ret = -EFAULT;
> 1157 goto done;
> 1158 }
> 1159 }
> 1160 pipe = usb_sndctrlpipe(dev, 0);
> 1161 snoop_urb(dev, NULL, pipe, ctrl->wLength, tmo, SUBMIT,
> 1162 tbuf, ctrl->wLength);
> 1163
> 1164 usb_unlock_device(dev);
> 1165 i = usb_control_msg(dev, usb_sndctrlpipe(dev, 0), ctrl->bRequest,
> 1166 ctrl->bRequestType, ctrl->wValue, ctrl->wIndex,
> 1167 tbuf, ctrl->wLength, tmo);
> 1168 usb_lock_device(dev);
> 1169 snoop_urb(dev, NULL, pipe, max(i, 0), min(i, 0), COMPLETE, NULL, 0);
> 1170 }
> 1171 if (i < 0 && i != -EPIPE) {
> 1172 dev_printk(KERN_DEBUG, &dev->dev, "usbfs: USBDEVFS_CONTROL "
> 1173 "failed cmd %s rqt %u rq %u len %u ret %d\n",
> 1174 current->comm, ctrl->bRequestType, ctrl->bRequest,
> 1175 ctrl->wLength, i);
> 1176 }
> 1177 ret = i;
> 1178 done:
> 1179 free_page((unsigned long) tbuf);
> 1180 usbfs_decrease_memory_usage(PAGE_SIZE + sizeof(struct urb) +
> 1181 sizeof(struct usb_ctrlrequest));
> 1182 return ret;
> 1183 }
> 1184
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
---end quoted text---
next prev parent reply other threads:[~2020-08-10 12:12 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-08 5:26 drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from_user() 'tbuf' too small (4096 vs 8192) kernel test robot
2020-08-08 5:26 ` kernel test robot
2020-08-10 12:12 ` Christoph Hellwig [this message]
2020-08-10 12:12 ` Christoph Hellwig
2020-08-10 12:14 ` Christoph Hellwig
2020-08-10 12:14 ` Christoph Hellwig
2020-08-10 13:34 ` Dan Carpenter
2020-08-10 13:34 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2021-01-24 15:50 kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200810121222.GA18639@lst.de \
--to=hch@lst.de \
--cc=kbuild-all@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.