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
| ||
|
Message-ID: <BYAPR07MB5381F90E3E2C4AE585F76520DD919@BYAPR07MB5381.namprd07.prod.outlook.com> Date: Wed, 10 Mar 2021 05:45:27 +0000 From: Pawel Laszczak <pawell@...ence.com> To: Peter Chen <hzpeterchen@...il.com> CC: "balbi@...nel.org" <balbi@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, USB list <linux-usb@...r.kernel.org>, lkml <linux-kernel@...r.kernel.org>, "laurent.pinchart@...asonboard.com" <laurent.pinchart@...asonboard.com> Subject: RE: [PATCH] usb: gadget: uvc: add bInterval checking for HS mode Please check whether the problem occurs in this fragment of code: https://elixir.bootlin.com/linux/latest/source/drivers/usb/cdns3/gadget.c#L2569 zlp_buff is allocated with kzalloc. Pawel >>On Fri, Mar 5, 2021 at 12:40 AM Pawel Laszczak <mailto:pawell@...ence.com> wrote: >>From: Pawel Laszczak <mailto:pawell@...ence.com> >> >>Patch adds extra checking for bInterval passed by configfs. >>The 5.6.4 chapter of USB Specification (rev. 2.0) say: >>"A high-bandwidth endpoint must specify a period of 1x125 µs >>(i.e., a bInterval value of 1)." >> >>The issue was observed during testing UVC class on CV. >>I treat this change as improvement because we can control >>bInterval by configfs. >> >>Signed-off-by: Pawel Laszczak <mailto:pawell@...ence.com> >>--- >> drivers/usb/gadget/function/f_uvc.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >>diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c >>index 44b4352a2676..5d62720bb9e1 100644 >>--- a/drivers/usb/gadget/function/f_uvc.c >>+++ b/drivers/usb/gadget/function/f_uvc.c >>@@ -631,6 +631,12 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) >> cpu_to_le16(min(opts->streaming_maxpacket, 1023U)); >> uvc_fs_streaming_ep.bInterval = opts->streaming_interval; >> >>+ /* A high-bandwidth endpoint must specify a bInterval value of 1 */ >>+ if (max_packet_mult > 1) >>+ uvc_hs_streaming_ep.bInterval = 1; >>+ else >>+ uvc_hs_streaming_ep.bInterval = opts->streaming_interval; >>+ >> >>There is a "uvc_hs_streaming_ep.bInterval = opts->streaming_interval;" again at below code >>Besides, the default value is 1 for opts->streaming_interval. What the real issue you observed >>at CV test? >> > >The issue occurs when I intentionally set: > echo 3072 > functions/$FUNCTION/streaming_maxpacket > echo 4 > functions/$FUNCTION/streaming_interval > >Then for CV CH9 TD 9.5: Endpoint Descriptor test it got: >"(Mult = 2)Illegal high speed isochronous endpoint MaxPacketSize : 0x400 >(USB: 1.2.78) A High speed Interrupt/Isochronous endpoint must have a MaxPacketSize between >683 and 1024 and bInterval value of 1 when the Mult value is two." > >For default value CV passed. Of course, I can fix it by changing streaming_interval, but I thought that >it could be good to have protection against this issue. >Especially since Usb 2 specification say that bInterval must be 1 for high bandwidth endpoints. > >Pawel > >>Peter >> >> uvc_hs_streaming_ep.wMaxPacketSize = >> cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11)); >> uvc_hs_streaming_ep.bInterval = opts->streaming_interval; >>-- >>2.25.1
Powered by blists - more mailing lists