netdev - Re: [PATCH net] packet: tpacket_v3: do not trigger bug() on wrong header status

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: <518AF306.8020307@redhat.com>
Date:	Thu, 09 May 2013 02:51:18 +0200
From:	Daniel Borkmann <dborkman@...hat.com>
To:	chetan loke <loke.chetan@...il.com>
CC:	David Miller <davem@...emloft.net>,
	netdev <netdev@...r.kernel.org>,
	Jakub Zawadzki <darkjames-ws@...kjames.pl>
Subject: Re: [PATCH net] packet: tpacket_v3: do not trigger bug() on wrong
 header status

On 05/08/2013 10:32 PM, chetan loke wrote:
> On Tue, May 7, 2013 at 4:49 PM, Daniel Borkmann <dborkman@...hat.com> wrote:
  [...]
>> So what's the point? If people see a WARN() in their kernel log they go
>> and complain on netdev that AF_PACKET is broken although its their
>> application?
>
> It's not complaining. Its called bug-reporting. Exactly, by removing
> the WARN you deprived the user from fixing him/her buggy application.

I'm sorry Chetan, I still don't see the justification. If the user does
crap, he'll get an -EINVAL when trying to setup an {R,T}X_RING wrongly.
How he access frames in his application and give them back to the kernel,
he will get from some docs. Do you really think that such users who write
crappy applications will care to look into the Linux kernel source code
to locate the warning and know that this might be in relation to their
code? Rather, in best case, they might send a bug report to netdev where
you have to tell them that it's not a kernel bug, but a bug in their user
space application.

Fact is, we currently do *not* warn him in case of TPACKET_V1 or TPACKET_V2
either, and yet it works for users! Neither do we warn in case of an
RX_RING nor TX_RING. So we *cannot* have double standards here, either we
do not warn at all or we do warn in *every* case. (What would give
justification to only WARN in TPACKET_V3 but not in TPACKET_V1 and
TPACKET_V2 while they all are based on the same underlying technology?)

And personally, I think the first case is more sane. Imho, the kernel is
not responsible to do *bug reports* to the user in this scenario. It makes
sense when you have syscalls and return an -EINVAL with possible cause
explanations in man pages, but we don't have this here. I think it doesn't
make sense to trigger a WARN instead, i.e. because it can also be triggered
maliciously/on purpose from user space. So the man-page of packet(7) would
then need a description saying that mis-usage will lead to kernel warnings?

Having that said, I also think that in af_packet.c's packet_set_ring() the
WARN with "Tx-ring is not supported." is too much and a simple -EINVAL or
better -EOPNOTSUPP or the like would have been sufficient. Currently we have
a WARN + -EINVAL for that case. But anyway, that's just a side note ...

If you think differently, then please go ahead and and put WARNs all over
the place for all other TPACKET versions as well and also for netlink's
mmap that is based on af_packet's mmap, which went in in 3.10, and also
document it in the man-page. I don't think this is reasonable.

Thanks,

Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html