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

[Thorsten Blum <thorsten.blum@linux.dev>]

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, ':');