All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Shoaib Rao <rao.shoaib@oracle.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>,
	linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
	andy.rudoff@intel.com
Subject: Re: [PATCH] af_unix: Allow Unix sockets to raise SIGURG
Date: Fri, 29 Jan 2021 12:18:37 -0800	[thread overview]
Message-ID: <20210129121837.467280fb@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <cef52fb0-43cb-9038-7e48-906b58b356b6@oracle.com>

On Fri, 29 Jan 2021 12:10:21 -0800 Shoaib Rao wrote:
> On 1/29/21 12:02 PM, Jakub Kicinski wrote:
> > On Fri, 29 Jan 2021 11:48:15 -0800 Shoaib Rao wrote:  
> >> Data was discarded because the flag was not supported, this patch
> >> changes that but does not support any urgent data.  
> > When you say it does not support any urgent data do you mean the
> > message len must be == 0 because something is checking it, or that
> > the code does not support its handling?
> >
> > I'm perfectly fine with the former, just point me at the check, please.  
> 
> The code does not care about the size of data -- All it does is that if 
> MSG_OOB is set it will deliver the signal to the peer process 
> irrespective of the length of the data (which can be zero length). Let's 
> look at the code of unix_stream_sendmsg() It does the following (sent is 
> initialized to zero)

Okay. Let me try again. AFAICS your code makes it so that data sent
with MSG_OOB is treated like any other data. It just sends a signal.
So you're hijacking the MSG_OOB to send a signal, because OOB also
sends a signal. But there is nothing OOB about the data itself. So 
I'm asking you to make sure that there is no data in the message. 
That way when someone wants _actual_ OOB data on UNIX sockets they 
can implement it without breaking backwards compatibility of the 
kernel uAPI.

> while (sent < len) {
>                  size = len - sent;
> <..>
> 
> }
> 
>          if (msg->msg_flags & MSG_OOB)
>                  sk_send_sigurg(other);
> 
> Before the patch there was a check above the while loop that checked the 
> flag and returned and error, that has been removed.

  reply	other threads:[~2021-01-29 20:19 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22 15:06 [PATCH] af_unix: Allow Unix sockets to raise SIGURG Matthew Wilcox (Oracle)
2021-01-25 23:36 ` Jakub Kicinski
2021-01-29 17:56   ` Shoaib Rao
2021-01-29 19:06     ` Jakub Kicinski
2021-01-29 19:48       ` Shoaib Rao
2021-01-29 20:02         ` Jakub Kicinski
2021-01-29 20:10           ` Shoaib Rao
2021-01-29 20:18             ` Jakub Kicinski [this message]
2021-01-29 20:44               ` Shoaib Rao
2021-01-29 20:49                 ` Shoaib Rao
2021-01-29 21:18                 ` Jakub Kicinski
2021-01-29 21:32                   ` Matthew Wilcox
2021-01-29 23:47                     ` David Laight
2021-01-29 21:54                   ` Shoaib Rao
2021-01-29 19:19     ` Matthew Wilcox
2021-01-29 19:54       ` Shoaib Rao
2021-01-29 20:01         ` Shoaib Rao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210129121837.467280fb@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=andy.rudoff@intel.com \
    --cc=davem@davemloft.net \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=rao.shoaib@oracle.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.