On Fri, May 31, 2024 at 11:45:48AM -0500, Nathan Lynch wrote: > Breno Leitao <lei...@debian.org> writes: > > > On Thu, May 30, 2024 at 07:44:12PM -0500, Nathan Lynch via B4 Relay wrote: > >> From: Nathan Lynch <nath...@linux.ibm.com>
> >> + nargs = array_index_nospec(nargs, ARRAY_SIZE(args.args)); > >> + nret = array_index_nospec(nret, ARRAY_SIZE(args.args) - nargs); > > > > On an unrelated note, can nargs and nret are integers and could be > > eventually negative. Is this a valid use case? > > No, it's not valid for a caller to provide negative nargs or nret. I > convinced myself that this bounds check: > > nargs = be32_to_cpu(args.nargs); > nret = be32_to_cpu(args.nret); > > if (nargs >= ARRAY_SIZE(args.args) > || nret > ARRAY_SIZE(args.args) > || nargs + nret > ARRAY_SIZE(args.args)) > return -EINVAL; > > rejects negative values of nargs or nret due to C's "usual arithmetic > conversions", where nargs and nret are implicitly converted to size_t > for the comparisons. > > However I don't see any value in keeping them as signed int. I have some > changes in progress in this area and I'll plan on making these unsigned. yea, I think it will help to make this code easier to read/review. Thanks again for fixing it.
