RE: [PATCH v2 net] net: dsa: microchip: provide a list of valid protocols for xmit handler

[Madhuri.Sripada]

Hi Sean,

> -----Original Message-----
> From: Sean Nyekjaer <sean@xxxxxxxxxx>
> Sent: Wednesday, December 6, 2023 12:47 PM
> To: Woojung Huh - C21699 <Woojung.Huh@xxxxxxxxxxxxx>;
> UNGLinuxDriver <UNGLinuxDriver@xxxxxxxxxxxxx>; Andrew Lunn
> <andrew@xxxxxxx>; Florian Fainelli <f.fainelli@xxxxxxxxx>; Vladimir Oltean
> <olteanv@xxxxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Arun Ramadoss - I17769
> <Arun.Ramadoss@xxxxxxxxxxxxx>; Christian Eggers <ceggers@xxxxxxx>
> Cc: Sean Nyekjaer <sean@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH v2 net] net: dsa: microchip: provide a list of valid protocols for
> xmit handler
> 
> [Some people who received this message don't often get email from
> sean@xxxxxxxxxx. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
> 
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Provide a list of valid protocols for which the driver will provide it's deferred
> xmit handler.
> 
> When using DSA_TAG_PROTO_KSZ8795 protocol, it does not provide a
> "connect" method, therefor ksz_connect() is not allocating ksz_tagger_data.
> 
> This avoids the following null pointer dereference:
>  ksz_connect_tag_protocol from dsa_register_switch+0x9ac/0xee0
> dsa_register_switch from ksz_switch_register+0x65c/0x828
> ksz_switch_register from ksz_spi_probe+0x11c/0x168  ksz_spi_probe from
> spi_probe+0x84/0xa8  spi_probe from really_probe+0xc8/0x2d8
> 
> Fixes: ab32f56a4100 ("net: dsa: microchip: ptp: add packet transmission
> timestamping")
> Signed-off-by: Sean Nyekjaer <sean@xxxxxxxxxx>
> ---
> https://lore.kernel.org/netdev/20231205124636.1345761-1-
> sean@xxxxxxxxxx/#R
> Changes since v1:
>  - Provided a list of valid protocols
> 
>  drivers/net/dsa/microchip/ksz_common.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/dsa/microchip/ksz_common.c
> b/drivers/net/dsa/microchip/ksz_common.c
> index 42db7679c360..286e20f340e5 100644
> --- a/drivers/net/dsa/microchip/ksz_common.c
> +++ b/drivers/net/dsa/microchip/ksz_common.c
> @@ -2624,10 +2624,18 @@ static int ksz_connect_tag_protocol(struct
> dsa_switch *ds,  {
>         struct ksz_tagger_data *tagger_data;
> 
> -       tagger_data = ksz_tagger_data(ds);
> -       tagger_data->xmit_work_fn = ksz_port_deferred_xmit;
> -
> -       return 0;
> +       switch (proto) {
> +       case DSA_TAG_PROTO_KSZ8795:
> +               return 0;
> +       case DSA_TAG_PROTO_KSZ9893:
> +       case DSA_TAG_PROTO_KSZ9477:
> +       case DSA_TAG_PROTO_LAN937X:
> +               tagger_data = ksz_tagger_data(ds);
> +               tagger_data->xmit_work_fn = ksz_port_deferred_xmit;


NULL check is missing here.

> +               return 0;
> +       default:
> +               return -EPROTONOSUPPORT;
> +       }
>  }
> 
>  static int ksz_port_vlan_filtering(struct dsa_switch *ds, int port,
> --
> 2.42.0