netdev - Re: [PATCH net-next] net: sctp: sctp_set_owner

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:	Fri, 09 Aug 2013 22:47:32 +0200
From:	Daniel Borkmann <dborkman@...hat.com>
To:	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org, linux-sctp@...r.kernel.org
Subject: Re: [PATCH net-next] net: sctp: sctp_set_owner_w: fix panic during
 skb orphaning

On 08/09/2013 10:37 PM, David Miller wrote:
> From: Daniel Borkmann <dborkman@...hat.com>
> Date: Wed,  7 Aug 2013 12:42:01 +0200
>
>> This patch fixes the following triggered bug ...
>   ...
>> ... that is triggered after commit 376c7311b ("net: add a temporary sanity
>> check in skb_orphan()"). What is happening is that we call sctp_set_owner_w()
>> for chunks in the SCTP output path from sctp_sendmsg(). Such chunks eventually
>> origin from constructors like sctp_make_chunk() where skb->sk = sk is being
>> set for socket accounting. Doing a git grep -n "skb->sk" net/sctp/ shows that
>> also in other places the socket pointer is being set, before issuing a
>> SCTP_CMD_SEND_PKT command and the like. Since SCTP is doing it's own memory
>> accounting anyway and has its own skb destructor functions, we should
>> customize sctp_set_owner_w() and call skb_orphan() if we set a different
>> owner of the skb than the current one in order to properly call their
>> destructor function, but not run into a panic due to our non-exisiting one as
>> we set sctp_wfree() destructor right after that. Otherwise, we can just skip
>> orphaning and reassignment to the very same socket and only set the destructor
>> handler.
>
> This debugging check is exactly trying to catch what SCTP is doing,
> setting skb->sk without also setting the destructor.
>
> I would much rather see you reorganize and fix SCTP to behave properly
> rather than coding up a check which is essentially "if skb_orphan() bug
> won't trigger, call it"  That defeats the whole purpose of the check.

Ok, will try to come up with a patch next week. Thanks.
--
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