linux-kernel - Re: [PATCH net] drivers/net/wan/lapbether: Added needed

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: <CAJht_EPGD1RmnU6-ZJYocXCY-qcPxXeEuurQ6GJod=WGO69-jg@mail.gmail.com>
Date:   Mon, 10 Aug 2020 11:13:46 -0700
From:   Xie He <xie.he.0141@...il.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Linux X25 <linux-x25@...r.kernel.org>,
        Martin Schiller <ms@....tdt.de>
Subject: Re: [PATCH net] drivers/net/wan/lapbether: Added needed_tailroom

On Mon, Aug 10, 2020 at 12:32 AM Willem de Bruijn
<willemdebruijn.kernel@...il.com> wrote:
>
> What happens when a tunnel device passes a packet to these devices?
> That will also not have allocated the extra tailroom. Does that cause
> a bug?

I looked at the code in net/ipv4/ip_tunnel.c. It indeed appeared to me
that it didn't take needed_tailroom into consideration. However it
does take needed_headroom into consideration through the macro
LL_RESERVED_SPACE. I think it would be better for it to take
needed_tailroom into consideration, too.

However, looking at the comment of needed_tailroom in
include/linux/netdevice.h, it says "Extra tailroom the hardware may
need, but not in all cases can this be guaranteed". So if we take this
comment as the spec, we can consider this to be not a bug. The reason
the author of this comment said so, might be that he wanted to add
needed_tailroom to solve some problems, but he was not able to change
all code to take needed_tailroom into consideration, so he wrote in
the comment saying that it was not necessary to always guarantee
needed_tailroom.

If we take this comment as the spec, to prevent bugs, any driver that
sets needed_tailroom must always check (and re-allocate if necessary)
before using the tailroom.

However, I still think it would be better to always take into
consideration needed_tailroom (and needed_headroom, too), so that
eventually we can remove the words of "but not in all cases can this
be guaranteed" from the comment. That would make the code more logical
and consistent.

Thank you for raising this important question about needed_tailroom!