Re: [PATCH net-next v2 1/2] keys, dns: drop unused upayload->data NUL terminator

Thorsten Blum
2026-04-13T00:31:52Z

On Sun, Apr 12, 2026 at 05:05:08PM -0700, Jakub Kicinski wrote:
> On Mon, 13 Apr 2026 01:04:54 +0200 Thorsten Blum wrote:
> > On Sun, Apr 12, 2026 at 02:10:04PM -0700, Jakub Kicinski wrote:
> > > On Fri, 10 Apr 2026 00:57:02 +0200 Thorsten Blum wrote:
> > > > In dns_resolver_preparse(), do not NUL-terminate ->data and allocate one
> > > > byte less. The NUL terminator is never used and only ->datalen bytes are
> > > > accessed.
> > >
> > > I can't see where this is used at all.
> > > Please write better commit messages, there's no way this 1 byte
> > > is worth the amount of time I wasted trying to review this :/
> >
> > The point of patch 1/2 is not the removed NUL terminator itself, but to
> > prepare for patch 2/2, which adds __counted_by() and requires ->datalen
> > to match the number of elements in ->data.
> >
> > Currently, that is not the case because ->data includes an extra NUL
> > despite never being used as a C string. Removing the unused terminator
> > makes the length match the allocation size and allows adding the
> > __counted_by() annotation.
> >
> > I can fold this into the __counted_by() patch if you prefer.
>
> I understand that part, but I don't get where the data from which
> the terminating character is removed, is used. Only other access
> I saw was freeing it, the rest of the callback seem to looking
> at the error, not the data..

->data and ->datalen are used in multiple places.

For example, in dns_query() in net/dns_resolver/dns_query.c:

upayload = user_key_payload_locked(rkey);
len = upayload->datalen;

if (_result) {
ret = -ENOMEM;
*_result = kmemdup_nul(upayload->data, len, GFP_KERNEL);
if (!*_result)
goto put;
}

In cifs_set_cifscreds() in fs/smb/client/connect.c:

/* find first : in payload */
payload = upayload->data;
delim = strnchr(payload, upayload->datalen, ':');