netdev - Re: Qualcomm rmnet driver and qmi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Jun 2018 19:43:52 +0200
From:   Bjørn Mork <bjorn@...k.no>
To:     Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
Cc:     Daniele Palmas <dnlplm@...il.com>, Dan Williams <dcbw@...hat.com>,
        netdev@...r.kernel.org
Subject: Re: Qualcomm rmnet driver and qmi_wwan

Subash Abhinov Kasiviswanathan <subashab@...eaurora.org> writes:

>> thanks, I will test it on Monday.
>>
>> Just a question for my knowledge: is the new sysfs attribute really
>> needed? I mean, is there not any other way to understand from qmi_wwan
>> without user intervention that there is the rmnet device attached?
>>
>> Regards,
>> Daniele
>>
>
> Hi Daniele
>
> You can check for the rx_handler attached to qmi_wwan dev and see if it
> belongs to rmnet. You can use the attached patch for it but it think the
> sysfs way might be a bit cleaner.
>
> -- 
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> From f7a2b90948da47ade1b345eddb37b721f5ab65f4 Mon Sep 17 00:00:00 2001
> From: Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
> Date: Sat, 9 Jun 2018 11:14:22 -0600
> Subject: [PATCH] net: qmi_wwan: Allow packets to pass through to rmnet
>
> Pass through mode is to allow packets in MAP format to be passed
> on to rmnet if the rmnet rx handler is attached to it.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
> ---
>  drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c |  4 +++-
>  drivers/net/usb/qmi_wwan.c                         | 10 ++++++++++
>  include/linux/if_rmnet.h                           | 20 ++++++++++++++++++++
>  3 files changed, 33 insertions(+), 1 deletion(-)
>  create mode 100644 include/linux/if_rmnet.h
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 5f4e447..164a18f 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/netlink.h>
>  #include <linux/netdevice.h>
> +#include <linux/if_rmnet.h>
>  #include "rmnet_config.h"
>  #include "rmnet_handlers.h"
>  #include "rmnet_vnd.h"
> @@ -48,10 +49,11 @@
>  	[IFLA_RMNET_FLAGS]	= { .len = sizeof(struct ifla_rmnet_flags) },
>  };
>  
> -static int rmnet_is_real_dev_registered(const struct net_device *real_dev)
> +int rmnet_is_real_dev_registered(const struct net_device *real_dev)
>  {
>  	return rcu_access_pointer(real_dev->rx_handler) == rmnet_rx_handler;
>  }
> +EXPORT_SYMBOL(rmnet_is_real_dev_registered);
>  
>  /* Needs rtnl lock */
>  static struct rmnet_port*
> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
> index f52a9be..abdae63 100644
> --- a/drivers/net/usb/qmi_wwan.c
> +++ b/drivers/net/usb/qmi_wwan.c
> @@ -22,6 +22,7 @@
>  #include <linux/usb/cdc.h>
>  #include <linux/usb/usbnet.h>
>  #include <linux/usb/cdc-wdm.h>
> +#include <linux/if_rmnet.h>
>  
>  /* This driver supports wwan (3G/LTE/?) devices using a vendor
>   * specific management protocol called Qualcomm MSM Interface (QMI) -
> @@ -354,6 +355,10 @@ static ssize_t add_mux_store(struct device *d,  struct device_attribute *attr, c
>  	if (kstrtou8(buf, 0, &mux_id))
>  		return -EINVAL;
>  
> +	/* rmnet is already attached here */
> +	if (rmnet_is_real_dev_registered(to_net_dev(d)))
> +		return -EINVAL;
> +


Maybe rmnet_is_real_dev_registered(dev->net) instead, since we use that
elsewhere in this function?


>  	/* mux_id [1 - 0x7f] range empirically found */
>  	if (mux_id < 1 || mux_id > 0x7f)
>  		return -EINVAL;
> @@ -543,6 +548,11 @@ static int qmi_wwan_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  	if (skb->len < dev->net->hard_header_len)
>  		return 0;
>  
> +	if (rawip && rmnet_is_real_dev_registered(skb->dev)) {
> +		skb->protocol = htons(ETH_P_MAP);
> +		return (netif_rx(skb) == NET_RX_SUCCESS);
> +	}

Like Daniele said: It would be good to have some way to know when the
rawip condition fails.  Or even better: Automatically force rawip mode
when the rmnet driver attaches.  But that doesn't seem possible?  No
notifications or anything when an rx handler is registered?

Hmm, looking at this I wonder: Is the rawip check really necessary?  You
skip all the extra rawip code in the driver anyway, so I don't see how
it matters.  But maybe the ethernet header_ops are a problem?

And I wonder about using skb->dev here.  Does that really work?  I
didn't think we set that until later.  Why not use dev->net instead?



Bjørn