Re: [PATCH] net: usbnet: Fix potential NULL pointer dereference

[Oliver Neukum]

On 02.11.23 10:06, Ren Mingshuai wrote:
> > > > 23ba07991dad said SKB can be NULL without describing the triggering
> > > > scenario. Always Check it before dereference to void potential NULL
> > > > pointer dereference.
> > > I've tried to find out the scenarios where SKB is NULL, but failed.
> > > It seems impossible for SKB to be NULL. If SKB can be NULL, please
> > > tell me the reason and I'd be very grateful.
> >
> > What do you mean? Grepping the function name shows call sites with NULL getting passed as skb.
>
> Yes And I just learned that during the cdc_ncm_driver.probe, it is possible to pass a NULL SKB to usbnet_start_xmit().

Hi,

yes it looks like NCM does funky things, but what does that mean?

ndp_to_end_store()

        /* flush pending data before changing flag */
        netif_tx_lock_bh(dev->net);
        usbnet_start_xmit(NULL, dev->net);
        spin_lock_bh(&ctx->mtx);
        if (enable)

expects some odd semantics from it. The proposed patch simply
increases the drop counter, which is by itself questionable, as
we drop nothing.

But it definitely does no IO, so we flush nothing.
That is, we clearly have bug(s) but the patch only papers over
them.
And frankly, the basic question needs to be answered:
Are you allowed to call ndo_start_xmit() with a NULL skb?

My understanding until now was that you must not.

	Regards
		Oliver