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]

Date:	Tue, 7 May 2013 12:59:53 -0400
From:	chetan loke <loke.chetan@...il.com>
To:	Daniel Borkmann <dborkman@...hat.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 Fri, May 3, 2013 at 8:57 AM, Daniel Borkmann <dborkman@...hat.com> wrote:
>
> Jakub reported that it is fairly easy to trigger the BUG() macro
> from user space with TPACKET_V3's RX_RING by just giving a wrong
> header status flag. We already had a similar situation in commit
> 7f5c3e3a80e6654 (``af_packet: remove BUG statement in
> tpacket_destruct_skb'') where this was the case in the TX_RING
> side that could be triggered from user space. So really, don't use
> BUG() or BUG_ON() unless there's really no way out, and i.e.
> don't use it for consistency checking when there's user space
> involved, no excuses, especially not if you're slapping the user
> with WARN + dump_stack + BUG all at once. The two functions are

remember, you were able to figure out what the problem was because of
the WARN and the forceful BUG.
Is BUG ok? Depends on how you look at it. If you are doing serious
network monitoring on a real life appliance
we don't really want folks to write crappy application. May be BUG is
too harsh. But then I don't see a WARN in this patch either (may be I
overlooked it).


> of concern:
>
>   prb_retire_current_block() [when block status != TP_STATUS_KERNEL]
>   prb_open_block() [when block_status != TP_STATUS_KERN

>
> Calls to prb_open_block() are guarded by ealier checks if block_status
> is really TP_STATUS_KERNEL (racy!), but the first one BUG() is easily
> triggable from user space. System behaves still stable after they are
> removed. Also remove that yoda condition entirely, since it's already
> guarded.

What yoda? It's called defensive programming. It's a single check and
not going to cost you anything.
--
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