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

Jakub Kicinski
2026-04-13T18:00:04Z

On Mon, 13 Apr 2026 02:31:46 +0200 Thorsten Blum wrote:
> 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:
> [...]
> [...]
> [...]
> > >
> > > 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, ':');
>

Alright, could you repost this after the merge window and CC David and
Jarkko on both patches? They supposedly maintain this.