[PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Kriish Sharma]

Fixes warnings observed during compilation with -Wformat-overflow:

drivers/net/wan/hdlc_ppp.c: In function ‘ppp_cp_event’:
drivers/net/wan/hdlc_ppp.c:353:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
  353 |                 netdev_info(dev, "%s down\n", proto_name(pid));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wan/hdlc_ppp.c:342:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
  342 |                 netdev_info(dev, "%s up\n", proto_name(pid));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Update proto_name() to return "LCP" by default instead of NULL.
This change silences the compiler without changing existing behavior
and removes the need for the local 'pname' variable in ppp_cp_event.

Suggested-by: Krzysztof Hałasa <khalasa@piap.pl>
Fixes: 262858079afd ("Add linux-next specific files for 20250926")
Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
---
v2:
  - Target the net tree with proper subject prefix "[PATCH net]"
  - Update proto_name() to return "LCP" by default instead of NULL
  - Remove local 'pname' variable in ppp_cp_event
  - Add Suggested-by tag for Krzysztof Hałasa

v1: https://lore.kernel.org/all/20251002180541.1375151-1-kriish.sharma2006@gmail.com/

 drivers/net/wan/hdlc_ppp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wan/hdlc_ppp.c b/drivers/net/wan/hdlc_ppp.c
index 7496a2e9a282..281699e8d799 100644
--- a/drivers/net/wan/hdlc_ppp.c
+++ b/drivers/net/wan/hdlc_ppp.c
@@ -133,7 +133,7 @@ static inline const char *proto_name(u16 pid)
 	case PID_IPV6CP:
 		return "IPV6CP";
 	default:
-		return NULL;
+		return "LCP";
 	}
 }
 
-- 
2.34.1

Re: [PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Simon Horman]

On Fri, Oct 03, 2025 at 09:29:18AM +0000, Kriish Sharma wrote:
> Fixes warnings observed during compilation with -Wformat-overflow:
> 
> drivers/net/wan/hdlc_ppp.c: In function ‘ppp_cp_event’:
> drivers/net/wan/hdlc_ppp.c:353:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
>   353 |                 netdev_info(dev, "%s down\n", proto_name(pid));
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/wan/hdlc_ppp.c:342:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
>   342 |                 netdev_info(dev, "%s up\n", proto_name(pid));
>       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Update proto_name() to return "LCP" by default instead of NULL.
> This change silences the compiler without changing existing behavior
> and removes the need for the local 'pname' variable in ppp_cp_event.
> 
> Suggested-by: Krzysztof Hałasa <khalasa@piap.pl>
> Fixes: 262858079afd ("Add linux-next specific files for 20250926")

Perhaps this should be:

Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC.")
But more importantly, and sorry for not noticing this in my review of v1,
I'm not sure this is a bug fix. In his review of v1 Chris explains that
this case cannot be hit. And that the patch is about silencing the
compiler.

If so, I'd suggest this is a clean-up and thus you should consider:
1) Removing the fixes tag
2) Retargeting the patch at net-next

Please also note that net-next is currently closed for the merge window.
So any patches for it should be sent after it reopens, which will
be after v6.18-rc1 is released, most likely on or after 13th October.

See: https://docs.kernel.org/process/maintainer-netdev.html

The code change itself looks good to me.

> Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
> ---
> v2:
>   - Target the net tree with proper subject prefix "[PATCH net]"
>   - Update proto_name() to return "LCP" by default instead of NULL
>   - Remove local 'pname' variable in ppp_cp_event
>   - Add Suggested-by tag for Krzysztof Hałasa
> 
> v1: https://lore.kernel.org/all/20251002180541.1375151-1-kriish.sharma2006@gmail.com/
> 

Re: [PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Kriish Sharma]

Hi Simon,

Thanks again for the clarification. I understand this is more of a
clean-up rather than a bug fix.
I’ll prepare a v3 targeting net-next once the merge window reopens,
removing the Fixes tag.

Best regards,
Kriish

On Fri, Oct 3, 2025 at 3:33 PM Simon Horman <horms@kernel.org> wrote:
>
> On Fri, Oct 03, 2025 at 09:29:18AM +0000, Kriish Sharma wrote:
> > Fixes warnings observed during compilation with -Wformat-overflow:
> >
> > drivers/net/wan/hdlc_ppp.c: In function ‘ppp_cp_event’:
> > drivers/net/wan/hdlc_ppp.c:353:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
> >   353 |                 netdev_info(dev, "%s down\n", proto_name(pid));
> >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > drivers/net/wan/hdlc_ppp.c:342:17: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
> >   342 |                 netdev_info(dev, "%s up\n", proto_name(pid));
> >       |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > Update proto_name() to return "LCP" by default instead of NULL.
> > This change silences the compiler without changing existing behavior
> > and removes the need for the local 'pname' variable in ppp_cp_event.
> >
> > Suggested-by: Krzysztof Hałasa <khalasa@piap.pl>
> > Fixes: 262858079afd ("Add linux-next specific files for 20250926")
>
> Perhaps this should be:
>
> Fixes: e022c2f07ae5 ("WAN: new synchronous PPP implementation for generic HDLC.")
> But more importantly, and sorry for not noticing this in my review of v1,
> I'm not sure this is a bug fix. In his review of v1 Chris explains that
> this case cannot be hit. And that the patch is about silencing the
> compiler.
>
> If so, I'd suggest this is a clean-up and thus you should consider:
> 1) Removing the fixes tag
> 2) Retargeting the patch at net-next
>
> Please also note that net-next is currently closed for the merge window.
> So any patches for it should be sent after it reopens, which will
> be after v6.18-rc1 is released, most likely on or after 13th October.
>
> See: https://docs.kernel.org/process/maintainer-netdev.html
>
> The code change itself looks good to me.
>
> > Signed-off-by: Kriish Sharma <kriish.sharma2006@gmail.com>
> > ---
> > v2:
> >   - Target the net tree with proper subject prefix "[PATCH net]"
> >   - Update proto_name() to return "LCP" by default instead of NULL
> >   - Remove local 'pname' variable in ppp_cp_event
> >   - Add Suggested-by tag for Krzysztof Hałasa
> >
> > v1: https://lore.kernel.org/all/20251002180541.1375151-1-kriish.sharma2006@gmail.com/
> >

Re: [PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Krzysztof Hałasa]

Hi Kriish,

Kriish Sharma <kriish.sharma2006@gmail.com> writes:

> --- a/drivers/net/wan/hdlc_ppp.c
> +++ b/drivers/net/wan/hdlc_ppp.c
> @@ -133,7 +133,7 @@ static inline const char *proto_name(u16 pid)
>         case PID_IPV6CP:
>                 return "IPV6CP";
>         default:
> -               return NULL;
> +               return "LCP";
>         }
>  }

I'd also remove the "PID_LCP" case as well (just those 2 lines
above IPCP case), the code will probably be simpler to understand
(the compiler won't care I guess).
-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

Re: [PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Kriish Sharma]

Hi Chris,

Thanks for the suggestion. That makes sense I’ll drop the PID_LCP case
as well in v3 to simplify the code.
I’ll resend the patch targeting net-next once the merge window reopens.

Best regards,
Kriish

On Fri, Oct 3, 2025 at 6:03 PM Krzysztof Hałasa <khalasa@piap.pl> wrote:
>
> Hi Kriish,
>
> Kriish Sharma <kriish.sharma2006@gmail.com> writes:
>
> > --- a/drivers/net/wan/hdlc_ppp.c
> > +++ b/drivers/net/wan/hdlc_ppp.c
> > @@ -133,7 +133,7 @@ static inline const char *proto_name(u16 pid)
> >         case PID_IPV6CP:
> >                 return "IPV6CP";
> >         default:
> > -               return NULL;
> > +               return "LCP";
> >         }
> >  }
>
> I'd also remove the "PID_LCP" case as well (just those 2 lines
> above IPCP case), the code will probably be simpler to understand
> (the compiler won't care I guess).
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa

Re: [PATCH net v2] hdlc_ppp: fix potential null pointer in ppp_cp_event logging

[Jakub Kicinski]

On Fri,  3 Oct 2025 09:29:18 +0000 Kriish Sharma wrote:
> Fixes: 262858079afd ("Add linux-next specific files for 20250926")

This is not a fix, please post the next version in 2 weeks
(after the merge window has closed) and without the Fixes tag. 

The warning (not so?) obviously doesn't apply to the kernel. 
Kernel print methods will output "(null)" if you pass 0 as 
the value to %s.

I defer to Krzysztof on whether we should proceed with this
patch as a cleanup, but -Wformat-overflow= is disabled in 
the kernel builds for a good reason. This is not a fix.
-- 
pw-bot: cr