Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1333081

Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

From Rainer Weikusat <rweikusat@mobileactivedefense.com>
Newsgroups linux.kernel
Subject Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg
Date 2016-02-12 21:20 +0100
Message-ID <r1sm5-3kI-9@gated-at.bofh.it> (permalink)
References (5 earlier) <r14jL-48t-5@gated-at.bofh.it> <r15fP-4NX-9@gated-at.bofh.it> <r1i3o-4XY-17@gated-at.bofh.it> <r1lXl-7yy-19@gated-at.bofh.it> <r1s2K-2YC-17@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Ben Hutchings <ben@decadent.org.uk> writes:
> On Fri, 2016-02-12 at 13:25 +0000, Rainer Weikusat wrote:
>> Philipp Hahn <pmhahn@pmhahn.de> writes:
>> > Hello Rainer,
>> > 
>> > Am 11.02.2016 um 20:37 schrieb Rainer Weikusat:
>> > > The unix_dgram_sendmsg routine use the following test
>> > > 
>> > > if (unlikely(unix_peer(other) != sk && unix_recvq_full(other))) {
>> 
>> [...]
>> 
>> > > This isn't correct as the> specified address could have been bound to
>> > > the sending socket itself
>> 
>> [...]
>> 
>> > After applying that patch at least my machine running the samba test no
>> > longer crashes.
>> 
>> There's a possible gotcha in there: Send-to-self used to be limited by
>> the queue limit. But the rationale for that (IIRC) was that someone
>> could keep using newly created sockets to queue ever more data to a
>> single, unrelated receiver. I don't think this should apply when
>> receiving and sending sockets are identical. But that's just my
>> opinion. The other option would be to avoid the unix_state_double_lock
>> for sk == other.
>
> Given that unix_state_double_lock() already handles sk == other, I'm
> not sure why you think it needs to be avoided.

Because the whole complication of restarting the operation after locking
both sk and other because other had to be unlocked before calling
unix_state_double_lock is useless for this case: As other == sk, there's
no reason to drop the lock on it which guarantees that the result of all
the earlier checks is still valid: If the -EAGAIN condition is not true,
execution can just continue.

>> I'd be willing to change this accordingly if someone
>> thinks the queue limit should apply to send-to-self.
>
> If we don't check the queue limit here, does anything else prevent the
> queue growing to the point it's a DoS?

The max_dgram_qlen limit exists specifically to prevent someone sending
'a lot' of messages to a socket unrelated to it by repeatedly creating a
socket, sending as many messages as the send buffer size will allow,
closing the socket, creating a new socket, ..., cf

http://netdev.vger.kernel.narkive.com/tcZIFJeC/get-rid-of-proc-sys-net-unix-max-dgram-qlen#post4
(first copy I found)

This 'attack' will obviously not work very well when sending and
receiving socket are identical.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: Bug 4.1.16: self-detected stall in net/unix/? Philipp Hahn <pmhahn@pmhahn.de> - 2016-02-11 14:50 +0100
  Re: Bug 4.1.16: self-detected stall in net/unix/? Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-11 17:00 +0100
    Re: Bug 4.1.16: self-detected stall in net/unix/? Ben Hutchings <ben@decadent.org.uk> - 2016-02-11 18:10 +0100
      Re: Bug 4.1.16: self-detected stall in net/unix/? Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-11 18:50 +0100
        Re: Bug 4.1.16: self-detected stall in net/unix/? Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-11 19:00 +0100
        Re: Bug 4.1.16: self-detected stall in net/unix/? Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-11 19:40 +0100
          [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-11 20:40 +0100
            Re: [PATCH net] af_unix: Guard against other == sk in  unix_dgram_sendmsg Philipp Hahn <pmhahn@pmhahn.de> - 2016-02-12 10:20 +0100
              Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-12 14:30 +0100
                Re: [PATCH net] af_unix: Guard against other == sk in  unix_dgram_sendmsg Ben Hutchings <ben@decadent.org.uk> - 2016-02-12 21:00 +0100
                Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-12 21:20 +0100
                Re: [PATCH net] af_unix: Guard against other == sk in  unix_dgram_sendmsg Ben Hutchings <ben@decadent.org.uk> - 2016-02-12 21:50 +0100
                Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2016-02-12 22:10 +0100
            Re: [PATCH net] af_unix: Guard against other == sk in  unix_dgram_sendmsg David Miller <davem@davemloft.net> - 2016-02-16 19:00 +0100

csiph-web