Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Sat, 28 Oct 2023 07:30:45 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Avichal Rakesh <arakesh@google.com>
Cc:     Dan Scally <dan.scally@ideasonboard.com>,
        laurent.pinchart@ideasonboard.com, etalvala@google.com,
        jchowdhary@google.com, linux-kernel@vger.kernel.org,
        linux-usb@vger.kernel.org, m.grzeschik@pengutronix.de
Subject: Re: [PATCH v8 2/4] usb: gadget: uvc: Allocate uvc_requests one at a
 time
Message-ID: <2023102823-embargo-envy-5fcb@gregkh>
References: <20231019185319.2714000-1-arakesh@google.com>
 <20231024183605.908253-1-arakesh@google.com>
 <20231024183605.908253-2-arakesh@google.com>
 <421d1996-8544-45ac-9f31-551ef597546c@ideasonboard.com>
 <1d3777ec-ac12-4a9a-a19c-fb7d5013a0a6@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1d3777ec-ac12-4a9a-a19c-fb7d5013a0a6@google.com>
Precedence: bulk

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