linux-kernel - Re: [PATCH v8 2/4] usb: gadget: uvc: Allocate uvc

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: <2023102823-embargo-envy-5fcb@gregkh>
Date:   Sat, 28 Oct 2023 07:30:45 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Avichal Rakesh <arakesh@...gle.com>
Cc:     Dan Scally <dan.scally@...asonboard.com>,
        laurent.pinchart@...asonboard.com, etalvala@...gle.com,
        jchowdhary@...gle.com, linux-kernel@...r.kernel.org,
        linux-usb@...r.kernel.org, m.grzeschik@...gutronix.de
Subject: Re: [PATCH v8 2/4] usb: gadget: uvc: Allocate uvc_requests one at a
 time

On Fri, Oct 27, 2023 at 01:31:26PM -0700, Avichal Rakesh wrote:
> >> @@ -322,6 +325,7 @@ uvc_video_free_requests(struct uvc_video *video)
> >>   static int
> >>   uvc_video_alloc_requests(struct uvc_video *video)
> >>   {
> >> +    struct uvc_request *ureq;
> >>       unsigned int req_size;
> >>       unsigned int i;
> >>       int ret = -ENOMEM;
> >> @@ -332,29 +336,34 @@ uvc_video_alloc_requests(struct uvc_video *video)
> >>            * max_t(unsigned int, video->ep->maxburst, 1)
> >>            * (video->ep->mult);
> >>
> >> -    video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
> >> -    if (video->ureq == NULL)
> >> -        return -ENOMEM;
> >> +    INIT_LIST_HEAD(&video->ureqs);
> > 
> > 
> > Probably unecessary here; it's done in uvc_video_free_requests() and uvcg_video_init() already
> 
> Ah, that is fair. Added a BUG_ON instead, like we do for video->req_size
> so we still catch cases where the state might be inconsistent.

Please no, that means you just crashed a machine and all data is lost
and the user will get very mad.

Either handle the error properly or it's something that can never happen
and so you don't need to handle it.

thanks,

greg k-h