netdev - Re: [PATCH net] qmi_wwan: Clone the skb when in pass-through mode

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:   Tue, 15 Jun 2021 12:04:21 +0200
From:   Bjørn Mork <bjorn@...k.no>
To:     Kristian Evensen <kristian.evensen@...il.com>
Cc:     Jakub Kicinski <kuba@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        subashab@...eaurora.org
Subject: Re: [PATCH net] qmi_wwan: Clone the skb when in pass-through mode

Kristian Evensen <kristian.evensen@...il.com> writes:

>> It does look pretty strange that qmimux_rx_fixup() copies out all
>> packets and receives them, and then let's usbnet to process the
>> multi-frame skb without even fulling off the qmimux_hdr. I'm probably
>> missing something.. otherwise sth like FLAG_MULTI_PACKET may be in
>> order?
>
> qmimux_rx_fixup() is different from what we are discussing here.
> qmimux_rx_fixup() is used when the de-aggregation is performed by the
> qmi_wwan driver, while the passthrough flag is set when the
> de-aggregation is done by the rmnet driver. The logic in
> qmimux_rx_fixup() is very similar to how the other usbnet mini-drivers
> handles de-aggregation and also how de-aggregation is handled by for
> example rmnet. I have no opinion on if the logic makes sens or not,
> but at least the origin can be traced :)

Yes, FLAG_MULTI_PACKET is only applicable to the qmimux case. But I
think Jakub is right that we should set it anyway. There is no way to
return from rx_fixup without an error or further processing of the skb,
unless we set FLAG_MULTI_PACKET.  Or invent something else.  But setting
that flag and then add the necessary usnet_sb_return call doesn't look
too bad?



Bjørn