Re: [PATCH RESEND] staging: nvec: use strcmp() instead of strncmp() with magic length

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]

On Tue, Jul 28, 2026 at 06:29:06PM +0300, Dan Carpenter wrote:
> On Tue, Jul 28, 2026 at 09:09:23AM +0200, Greg KH wrote:
> > On Thu, Jul 23, 2026 at 07:18:23PM +0300, Artem Lytkin wrote:
> > > Replace strncmp() with a hardcoded length of 30 with strcmp().
> > 
> > I thought we were trying to get rid of strcmp() usage?  Why add it
> > back?
> 
> I don't think we're trying to get rid of strcmp().
> 
> strncmp() is for prefixes and and strcmp() is for whole words.
> They're not the same.  strncmp() with a fixed string always
> seemed like nonsense to me and it causes a static checker in
> unpublished Smatch checks because we've seen that bug where
> people wanted to check the prefix but used the wrong number of
> characters.

Maybe the commit message should be updated. It makes the argument that
bat_type is NUL-terminated and that that is the reason why changing this
to strcmp() is safe. But it's not. There doesn't seem to be any sanity
checking on the EC data that serves as input to this, so the length of
the string that is copied could be any arbitrary value for all we know,
meaning we may already be writing past the end of bat_type and insert
the NUL some place invalid to begin with (we should probably fix that at
some point).

The real reason the conversion is safe is because strcmp() stops
searching the haystack if the needle is exhausted. Since the needle is
only 2 characters in this case, *that's* why we don't have to worry
about the limiting the search.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]