On Mon, Feb 27, 2023 at 7:32 PM Leon Romanovsky [off-list ref] wrote:

On Mon, Feb 27, 2023 at 10:55:04AM +0100, Aleksandr Mikhalitsyn wrote:

quoted

On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky [off-list ref] wrote:

quoted

On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote:

quoted

Currently, we set MSG_CTRUNC flag is we have no
msg_control buffer provided and SO_PASSCRED is set
or if we have pending SCM_RIGHTS.

For some reason we have no corresponding check for
SO_PASSSEC.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Alexander Mikhalitsyn <redacted>
---
 include/net/scm.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Is it a bugfix? If yes, it needs Fixes line.

It's from 1da177e4c3 ("Linux-2.6.12-rc2") times :)
I wasn't sure that it's correct to put the "Fixes" tag on such an old
and big commit. Will do. Thanks!

quoted

quoted

diff --git a/include/net/scm.h b/include/net/scm.h
index 1ce365f4c256..585adc1346bd 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h

@@ -105,16 +105,27 @@ static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct sc
              }
      }
 }
+
+static inline bool scm_has_secdata(struct socket *sock)
+{
+     return test_bit(SOCK_PASSSEC, &sock->flags);
+}
 #else
 static inline void scm_passec(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
 { }
+
+static inline bool scm_has_secdata(struct socket *sock)
+{
+     return false;
+}
 #endif /* CONFIG_SECURITY_NETWORK */

There is no need in this ifdef, just test bit directly.

The problem is that even if the kernel is compiled without
CONFIG_SECURITY_NETWORK
userspace can still set the SO_PASSSEC option. IMHO it's better not to
set MSG_CTRUNC
if CONFIG_SECURITY_NETWORK is disabled, msg_control is not set but
SO_PASSSEC is enabled.
Because in this case SCM_SECURITY will never be sent. Please correct
me if I'm wrong.

I don't know enough in this area to say if it is wrong or not.
My remark was due to the situation where user sets some bit which is
going to be ignored silently. It will be much cleaner do not set it
if CONFIG_SECURITY_NETWORK is disabled instead of masking its usage.

Hi Leon,

I agree with you, but IMHO then it looks more correct to return -EOPNOTSUPP on
setsockopt(fd, SO_PASSSEC, ...) if CONFIG_SECURITY_NETWORK is disabled.
But such a change may break things.

Okay, anyway I'll wait until net-next will be opened and present a
patch with a more
detailed description and Fixes tag. Speaking about this problem with
CONFIG_SECURITY_NETWORK
if you insist that it will be more correct then I'm ready to fix it too.

Thanks,
Alex

Thanks

quoted

Kind regards,
Alex

quoted

quoted

 static __inline__ void scm_recv(struct socket *sock, struct msghdr *msg,
                              struct scm_cookie *scm, int flags)
 {
      if (!msg->msg_control) {
-             if (test_bit(SOCK_PASSCRED, &sock->flags) || scm->fp)
+             if (test_bit(SOCK_PASSCRED, &sock->flags) || scm->fp ||
+                 scm_has_secdata(sock))
                      msg->msg_flags |= MSG_CTRUNC;
              scm_destroy(scm);
              return;
--
2.34.1