netdev - Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom

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]

Message-ID: <20220517061221.012e36d1.max@enpas.org>
Date:   Tue, 17 May 2022 06:12:21 +0200
From:   Max Staudt <max@...as.org>
To:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>
Cc:     Oliver Hartkopp <socketcan@...tkopp.net>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-can@...r.kernel.org, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and
 can_skb_headroom_valid to skb.c

On Tue, 17 May 2022 10:50:16 +0900
Vincent MAILHOL <mailhol.vincent@...adoo.fr> wrote:

> My concern is: what would be the merrit? If we do not split, the users
> of slcan and v(x)can would have to load the can-dev module which will
> be slightly bloated for their use, but is this really an issue? I do
> not see how this can become a performance bottleneck, so what is the
> problem?
> I could also argue that most of the devices do not depend on
> rx-offload.o. So should we also split this one out of can-dev on the
> same basis and add another module dependency?
> The benefit (not having to load a bloated module for three drivers)
> does not outweigh the added complexity: all hardware modules will have
> one additional modprobe dependency on the tiny can-skb module.
> 
> But as said above, I am not fully opposed to the split, I am just
> strongly divided. If we go for the split, creating a can-skb module is
> the natural and only option I see.
> If the above argument does not convince you, I will send a v3 with
> that split.

I originally replied to PATCHv2 in agreement with what Vincent said in
the first part - I agree that simply moving this code into can-dev and
making v(x)can/slcan depend on it is the straightforward thing to do.

Having yet another kernel module for a tiny bit of code adds more
unnecessary complexity IMHO. And from a user perspective, it seems
fairly natural to have can-dev loaded any time some can0, slcan0,
vcan0, etc. pops up.

Finally, embedded applications that are truly short on memory are
likely using a "real" CAN adapter, and hence already have can-dev
loaded anyway.

I think it's okay to just move it to can-dev and call it a day :)


Max