On 15/09/2026 22:52, Sriram Sriram wrote: > From: Drew Kluemke <[email protected]> > > The OACK handler matches option names with strcasecmp() and parses the > values with dectoul(). Both scan until a NUL byte, so a malformed or > truncated OACK that contains no NUL within the received length makes > them read past the end of the packet buffer. > > RFC 2347 formats an OACK as a sequence of NUL terminated netascii > strings, so the last byte of a well-formed packet is always a NUL. > Check for it once on entry and drop the packet if it is missing, which > keeps every strcasecmp() and dectoul() below in bounds without having to > bound each one individually. > > Signed-off-by: Drew Kluemke <[email protected]> > Signed-off-by: Sriram Sriram <[email protected]> > --- > Changes in v2: > - Replaced the four bounded strncasecmp() comparisons with a single > check that the packet ends in a NUL, per Tom's review. RFC 2347 > guarantees the trailing NUL, so one test covers every strcasecmp() > and, unlike v1, the dectoul() calls as well. > - Smaller: +16 bytes of .text on am335x_evm_defconfig, against +24 for > v1 (net/tftp.o 3222 unpatched, 3246 in v1, 3238 here). > - Retitled from "net: tftp: use bounded string compare for OACK option > parsing" to match the new approach. > > net/tftp.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/net/tftp.c b/net/tftp.c > index beb9d08f5a2..73b31b8357d 100644 > --- a/net/tftp.c > +++ b/net/tftp.c > @@ -528,6 +528,13 @@ static void tftp_handler(uchar *pkt, unsigned dest, > struct in_addr sip, > #endif > > case TFTP_OACK: > + /* > + * RFC 2347 makes every option name and value a NUL terminated > + * string, so a well-formed OACK always ends in a NUL. Checking > + * that once keeps the option parsing below in bounds. > + */ > + if (!len || pkt[len - 1] != '\0') > + return; > debug("Got OACK: "); > for (i = 0; i < len; i++) { > if (pkt[i] == '\0')
Reviewed-by: Jerome Forissier <[email protected]> Thanks, -- Jerome
