Re: [PATCH v2] ACPI: NFIT: prevent underflow in acpi_nfit_ctl()

[Dan Carpenter <error27@gmail.com>]

On Tue, Mar 28, 2023 at 05:55:40PM +0300, Dan Carpenter wrote:
> On Fri, Jan 20, 2023 at 08:22:06AM +0300, Dan Carpenter wrote:
> > On Thu, Jan 19, 2023 at 11:21:22AM -0500, Jeff Moyer wrote:
> > > Dan Carpenter <error27@gmail.com> writes:
> > > 
> > > > The concern here would be that "family" is negative and we pass a
> > > > negative value to test_bit() resulting in an out of bounds read
> > > > and potentially a crash.
> > > 
> > > I don't see how this can happen.  Do you have a particular scenario in
> > > mind?
> > > 
> > 
> > This is from static analysis.  My main thinking was:
> > 
> > 1) The static checker says that this comes from the user.
> > 2) Every upper bounds check should have a lower bounds check.
> > 3) family is passed to array_index_nospec() so we must not trust it.
> > 
> > But looking closer today here is what the checker is concerned about:
> > 
> > 	func = cmd_to_func(nfit_mem, cmd, call_pkg, &family);
> > 
> > Assume "nfit_mem" is NULL but "call_pkg" is non NULL (user input from
> > __nd_ioctl() or ars_get_status().  In that case family is unchecked user
> > input.
> > 
> > But probably, it's not possible for nfit_mem to be NULL in those caller
> > functions?
> 
> Did we ever figure out if it's possible for nfit_mem to be NULL?

Another idea is I could send this patch as a static checker fix instead
of a security vulnerability.  That way we would be safe going forward
regardless.

regards,
dan carpenter