netdev - Re: [PATCH net] sctp: fix a success return may hide an error

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] [day] [month] [year] [list]

Message-ID: <20160818174423.GG3110@localhost.localdomain>
Date:   Thu, 18 Aug 2016 14:44:23 -0300
From:   "'Marcelo Ricardo Leitner'" <marcelo.leitner@...il.com>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     "'Xin Long'" <lucien.xin@...il.com>,
        David Miller <davem@...emloft.net>,
        network dev <netdev@...r.kernel.org>,
        "linux-sctp@...r.kernel.org" <linux-sctp@...r.kernel.org>,
        Vladislav Yasevich <vyasevich@...il.com>,
        "daniel@...earbox.net" <daniel@...earbox.net>
Subject: Re: [PATCH net] sctp: fix a success return may hide an error

On Wed, Aug 17, 2016 at 09:01:38AM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 16 August 2016 18:25
> ...
> > > That doesn't seem a good idea.
> > > You don't want to abort the association if there is a transient
> > > memory allocation failure.
> > > You also can't drop data chunks.
> > 
> > From a system-wise POV, this behavior - to free the new asoc in case of
> > transient memory allocation failure - doesn't seem bad to me.
> > That's what will have to happen if any allocation before it failed and
> > also it helps the system to reduce the stress a little bit. I don't see
> > any inconsistency/problems here because we are not dropping a single
> > random chunk but instead we are actually refusing to initialize a new
> > asoc in such conditions.
> 
> Failing a new association should be ok, whether purists will like
> connect() failing ENOMEM is another matter.
> 

Good point.

> > Nevertheless, I agree that letting the application see ENOMEM errors when
> > the data actually got queued and is being fully handled, as in, it will
> > be retransmitted later, is not be wise, as the application probably
> > won't be able to distinguish from ENOMEMs that it should retry or not.
> 
> I think an application would be justified in thinking that an ENOMEM return
> meant that the system call had no effect.
> 

Yep

> For send() even ENOMEM is really wrong, it should be treated as 'flow control'
> and either block or return EAGAIN/EWOULDBLOCK.

Agreed.

> Getting POLLOUT set is left as an exercise to the reader :-)
> 

:-)

> ...
> > Well, it may be, but we are trying to improve it.  Please continue
> > discussing the fixes so we can keep improving it. :)
> 
> Indeed, we have customers who use sctp (for M3UA).
> We don't do anything 'complicated', but do end up sending a lot of short
> data chunks.
> 
> 	David
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>