From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754900AbbKMTGn (ORCPT ); Fri, 13 Nov 2015 14:06:43 -0500 Received: from tiger.mobileactivedefense.com ([217.174.251.109]:39804 "EHLO tiger.mobileactivedefense.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751934AbbKMTGl (ORCPT ); Fri, 13 Nov 2015 14:06:41 -0500 From: Rainer Weikusat To: Hannes Frederic Sowa Cc: Rainer Weikusat , Jason Baron , Dmitry Vyukov , syzkaller , Michal Kubecek , Al Viro , linux-fsdevel@vger.kernel.org, LKML , David Miller , David Howells , Paul Moore , salyzyn@android.com, sds@tycho.nsa.gov, ying.xue@windriver.com, netdev , Kostya Serebryany , Alexander Potapenko , Andrey Konovalov , Sasha Levin , Julien Tinnes , Kees Cook , Mathias Krause Subject: Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue In-Reply-To: <1447267928.3704532.436332953.6FB2A816@webmail.messagingengine.com> (Hannes Frederic Sowa's message of "Wed, 11 Nov 2015 19:52:08 +0100") References: <20151012120249.GB16370@unicorn.suse.cz> <1444652071.27760.156.camel@edumazet-glaptop2.roam.corp.google.com> <563CC002.5050307@akamai.com> <87ziyrcg67.fsf@doppelsaurus.mobileactivedefense.com> <87fv0fnslr.fsf_-_@doppelsaurus.mobileactivedefense.com> <877flp34fl.fsf@doppelsaurus.mobileactivedefense.com> <1447244898.1936942.435925969.525D20D9@webmail.messagingengine.com> <87ziyk347o.fsf@doppelsaurus.mobileactivedefense.com> <1447267928.3704532.436332953.6FB2A816@webmail.messagingengine.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.4 (gnu/linux) Date: Fri, 13 Nov 2015 19:06:11 +0000 Message-ID: <876115sorg.fsf@doppelsaurus.mobileactivedefense.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.4.3 (tiger.mobileactivedefense.com [217.174.251.109]); Fri, 13 Nov 2015 19:06:20 +0000 (GMT) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hannes Frederic Sowa writes: > On Wed, Nov 11, 2015, at 17:12, Rainer Weikusat wrote: >> Hannes Frederic Sowa writes: >> > On Tue, Nov 10, 2015, at 22:55, Rainer Weikusat wrote: >> >> An AF_UNIX datagram socket being the client in an n:1 association with >> >> some server socket is only allowed to send messages to the server if the >> >> receive queue of this socket contains at most sk_max_ack_backlog >> >> datagrams. >> >> [...] >> >> > This whole patch seems pretty complicated to me. >> > >> > Can't we just remove the unix_recvq_full checks alltogether and unify >> > unix_dgram_poll with unix_poll? >> > >> > If we want to be cautious we could simply make unix_max_dgram_qlen limit >> > the number of skbs which are in flight from a sending socket. The skb >> > destructor can then decrement this. This seems much simpler. >> > >> > Would this work? >> >> In the way this is intended to work, cf >> >> http://marc.info/?t=115627606000002&r=1&w=2 > > Oh, I see, we don't limit closed but still referenced sockets. This > actually makes sense on how fd handling is implemented, just as a range > check. > > Have you checked if we can somehow deregister the socket in the poll > event framework? You wrote that it does not provide such a function but > maybe it would be easy to add? I thought about this but this would amount to adding a general interface for the sole purpose of enabling the af_unix code to talk to the eventpoll code and I don't really like this idea: IMHO, there should be at least two users (preferably three) before creating any kind of 'abstract interface'. An even more ideal "castle in the air" (hypothetical) solution would be "change the eventpoll.c code such that it won't be affected if a wait queue just goes away". That's at least theoretically possible (although it might not be in practice). I wouldn't mind doing that (assuming it was possible) if it was just for the kernels my employer uses because I'm aware of the uses these will be put to and in control of the corresponding userland code. But for "general Linux code", changing epoll in order to help the af_unix code is more potential trouble than it's worth: Exchanging a relatively unimportant bug in some module for a much more visibly damaging bug in a central facility would be a bad tradeoff.