linux-kernel - Re: drivers/usb/core/devio.c:1155 do_proc_control() error: copy_from

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200810121222.GA18639@lst.de>
Date:   Mon, 10 Aug 2020 14:12:23 +0200
From:   Christoph Hellwig <hch@....de>
To:     kernel test robot <lkp@...el.com>
Cc:     Christoph Hellwig <hch@....de>, kbuild-all@...ts.01.org,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: drivers/usb/core/devio.c:1155 do_proc_control() error:
 copy_from_user() 'tbuf' too small (4096 vs 8192)

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@...el.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---