net: cdc_ncm: initialize drvflags before usage

net: cdc_ncm: initialize drvflags before usage

[Porto Rio]

Hi all, 

we detected a problem in stable Kernel 4.4.114 in drivers/net/usb/cdc_ncm.c. In line 833, ctx->drvflags is checked in the if clause:

	if (ctx->drvflags & CDC_NCM_FLAG_RESET_NTB16) {

but it is initialized *later* in line 877:

	/* Device-specific flags */
	ctx->drvflags = drvflags;

This initialization has to be done before the if clause.

Note, that the if clause was backported from mainline at Nov. 15th 2017 (GetNtbFormat endian fix). In mainline, the initialization is at the right place before the if clause.

Please find here a suggested patch:



--- linux/drivers/net/usb/cdc_ncm.c.orig	2018-02-01 13:55:20.034393993 +0100
+++ linux/drivers/net/usb/cdc_ncm.c	2018-02-01 13:56:12.842393881 +0100
@@ -825,6 +825,9 @@ int cdc_ncm_bind_common(struct usbnet *d
 		goto error2;
 	}
 
+	/* Device-specific flags */
+	ctx->drvflags = drvflags;
+
 	/*
 	 * Some Huawei devices have been observed to come out of reset in NDP32 mode.
 	 * Let's check if this is the case, and set the device to NDP16 mode again if
@@ -873,9 +876,6 @@ int cdc_ncm_bind_common(struct usbnet *d
 	/* finish setting up the device specific data */
 	cdc_ncm_setup(dev);
 
-	/* Device-specific flags */
-	ctx->drvflags = drvflags;
-
 	/* Allocate the delayed NDP if needed. */
 	if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END) {
 		ctx->delayed_ndp16 = kzalloc(ctx->max_ndp_size, GFP_KERNEL);

Re: net: cdc_ncm: initialize drvflags before usage

[Greg KH]

On Thu, Feb 01, 2018 at 02:49:38PM +0100, Porto Rio wrote:
> Hi all, 
> 
> we detected a problem in stable Kernel 4.4.114 in drivers/net/usb/cdc_ncm.c. In line 833, ctx->drvflags is checked in the if clause:
> 
> 	if (ctx->drvflags & CDC_NCM_FLAG_RESET_NTB16) {
> 
> but it is initialized *later* in line 877:
> 
> 	/* Device-specific flags */
> 	ctx->drvflags = drvflags;
> 
> This initialization has to be done before the if clause.
> 
> Note, that the if clause was backported from mainline at Nov. 15th 2017 (GetNtbFormat endian fix). In mainline, the initialization is at the right place before the if clause.
> 
> Please find here a suggested patch:
> 
> 
> 
> --- linux/drivers/net/usb/cdc_ncm.c.orig	2018-02-01 13:55:20.034393993 +0100
> +++ linux/drivers/net/usb/cdc_ncm.c	2018-02-01 13:56:12.842393881 +0100
> @@ -825,6 +825,9 @@ int cdc_ncm_bind_common(struct usbnet *d
>  		goto error2;
>  	}
>  
> +	/* Device-specific flags */
> +	ctx->drvflags = drvflags;
> +
>  	/*
>  	 * Some Huawei devices have been observed to come out of reset in NDP32 mode.
>  	 * Let's check if this is the case, and set the device to NDP16 mode again if
> @@ -873,9 +876,6 @@ int cdc_ncm_bind_common(struct usbnet *d
>  	/* finish setting up the device specific data */
>  	cdc_ncm_setup(dev);
>  
> -	/* Device-specific flags */
> -	ctx->drvflags = drvflags;
> -
>  	/* Allocate the delayed NDP if needed. */
>  	if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END) {
>  		ctx->delayed_ndp16 = kzalloc(ctx->max_ndp_size, GFP_KERNEL);

Nice catch, care to resend this in a format I can apply it in, with a
signed-off-by and everything?

thanks,

greg k-h