* [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC @ 2023-02-26 20:17 Alexander Mikhalitsyn 2023-02-27 9:47 ` Leon Romanovsky 2023-02-27 10:01 ` Eric Dumazet 0 siblings, 2 replies; 9+ messages in thread From: Alexander Mikhalitsyn @ 2023-02-26 20:17 UTC (permalink / raw) To: davem Cc: linux-kernel, netdev, Alexander Mikhalitsyn, Eric Dumazet, Jakub Kicinski, Paolo Abeni 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 <aleksandr.mikhalitsyn@canonical.com> --- include/net/scm.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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 */ 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-26 20:17 [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC Alexander Mikhalitsyn @ 2023-02-27 9:47 ` Leon Romanovsky 2023-02-27 9:55 ` Aleksandr Mikhalitsyn 2023-02-27 10:01 ` Eric Dumazet 1 sibling, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2023-02-27 9:47 UTC (permalink / raw) To: Alexander Mikhalitsyn Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > 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 <aleksandr.mikhalitsyn@canonical.com> > --- > include/net/scm.h | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) Is it a bugfix? If yes, it needs Fixes line. > > 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. > > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-27 9:47 ` Leon Romanovsky @ 2023-02-27 9:55 ` Aleksandr Mikhalitsyn 2023-02-27 18:32 ` Leon Romanovsky 0 siblings, 1 reply; 9+ messages in thread From: Aleksandr Mikhalitsyn @ 2023-02-27 9:55 UTC (permalink / raw) To: Leon Romanovsky Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky <leon@kernel.org> wrote: > > On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > > 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 <aleksandr.mikhalitsyn@canonical.com> > > --- > > 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! > > > > > 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. Kind regards, Alex > > > > > 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 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-27 9:55 ` Aleksandr Mikhalitsyn @ 2023-02-27 18:32 ` Leon Romanovsky 2023-02-28 10:06 ` Aleksandr Mikhalitsyn 0 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2023-02-27 18:32 UTC (permalink / raw) To: Aleksandr Mikhalitsyn Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Mon, Feb 27, 2023 at 10:55:04AM +0100, Aleksandr Mikhalitsyn wrote: > On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > > > 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 <aleksandr.mikhalitsyn@canonical.com> > > > --- > > > 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! > > > > > > > > > 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. Thanks > > Kind regards, > Alex > > > > > > > > > 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 > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-27 18:32 ` Leon Romanovsky @ 2023-02-28 10:06 ` Aleksandr Mikhalitsyn 2023-02-28 14:45 ` Leon Romanovsky 0 siblings, 1 reply; 9+ messages in thread From: Aleksandr Mikhalitsyn @ 2023-02-28 10:06 UTC (permalink / raw) To: Leon Romanovsky Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Mon, Feb 27, 2023 at 7:32 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Mon, Feb 27, 2023 at 10:55:04AM +0100, Aleksandr Mikhalitsyn wrote: > > On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > > > On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > > > > 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 <aleksandr.mikhalitsyn@canonical.com> > > > > --- > > > > 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! > > > > > > > > > > > > > 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 > > > > > Kind regards, > > Alex > > > > > > > > > > > > > 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 > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-28 10:06 ` Aleksandr Mikhalitsyn @ 2023-02-28 14:45 ` Leon Romanovsky 2023-02-28 15:10 ` Aleksandr Mikhalitsyn 0 siblings, 1 reply; 9+ messages in thread From: Leon Romanovsky @ 2023-02-28 14:45 UTC (permalink / raw) To: Aleksandr Mikhalitsyn Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Tue, Feb 28, 2023 at 11:06:12AM +0100, Aleksandr Mikhalitsyn wrote: > On Mon, Feb 27, 2023 at 7:32 PM Leon Romanovsky <leon@kernel.org> wrote: > > > > On Mon, Feb 27, 2023 at 10:55:04AM +0100, Aleksandr Mikhalitsyn wrote: > > > On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > > > > > On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > > > > > 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 <aleksandr.mikhalitsyn@canonical.com> > > > > > --- > > > > > 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! > > > > > > > > > > > > > > > > > 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. I won't insist on anything, most likely Eric will comment if you need to fix it. Thanks > > Thanks, > Alex > > > > > Thanks > > > > > > > > Kind regards, > > > Alex > > > > > > > > > > > > > > > > > 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 > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-28 14:45 ` Leon Romanovsky @ 2023-02-28 15:10 ` Aleksandr Mikhalitsyn 0 siblings, 0 replies; 9+ messages in thread From: Aleksandr Mikhalitsyn @ 2023-02-28 15:10 UTC (permalink / raw) To: Leon Romanovsky Cc: davem, linux-kernel, netdev, Eric Dumazet, Jakub Kicinski, Paolo Abeni On Tue, Feb 28, 2023 at 3:45 PM Leon Romanovsky <leon@kernel.org> wrote: > > On Tue, Feb 28, 2023 at 11:06:12AM +0100, Aleksandr Mikhalitsyn wrote: > > On Mon, Feb 27, 2023 at 7:32 PM Leon Romanovsky <leon@kernel.org> wrote: > > > > > > On Mon, Feb 27, 2023 at 10:55:04AM +0100, Aleksandr Mikhalitsyn wrote: > > > > On Mon, Feb 27, 2023 at 10:47 AM Leon Romanovsky <leon@kernel.org> wrote: > > > > > > > > > > On Sun, Feb 26, 2023 at 09:17:30PM +0100, Alexander Mikhalitsyn wrote: > > > > > > 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 <aleksandr.mikhalitsyn@canonical.com> > > > > > > --- > > > > > > 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! > > > > > > > > > > > > > > > > > > > > > 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. > > I won't insist on anything, most likely Eric will comment if you need to > fix it. Got it. Thanks a lot for your attention to the patch! Kind regards, Alex > > Thanks > > > > > Thanks, > > Alex > > > > > > > > Thanks > > > > > > > > > > > Kind regards, > > > > Alex > > > > > > > > > > > > > > > > > > > > > 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 > > > > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-26 20:17 [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC Alexander Mikhalitsyn 2023-02-27 9:47 ` Leon Romanovsky @ 2023-02-27 10:01 ` Eric Dumazet 2023-02-27 10:21 ` Aleksandr Mikhalitsyn 1 sibling, 1 reply; 9+ messages in thread From: Eric Dumazet @ 2023-02-27 10:01 UTC (permalink / raw) To: Alexander Mikhalitsyn Cc: davem, linux-kernel, netdev, Jakub Kicinski, Paolo Abeni On Sun, Feb 26, 2023 at 9:17 PM Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com> wrote: > > 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. Can you describe what side effects this patch has ? I think it could break some applications, who might not be able to recover from MSG_CTRUNC in this case. This should be documented, in order to avoid a future revert. In any case, net-next is currently closed. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC 2023-02-27 10:01 ` Eric Dumazet @ 2023-02-27 10:21 ` Aleksandr Mikhalitsyn 0 siblings, 0 replies; 9+ messages in thread From: Aleksandr Mikhalitsyn @ 2023-02-27 10:21 UTC (permalink / raw) To: Eric Dumazet; +Cc: davem, linux-kernel, netdev, Jakub Kicinski, Paolo Abeni On Mon, Feb 27, 2023 at 11:01 AM Eric Dumazet <edumazet@google.com> wrote: > > On Sun, Feb 26, 2023 at 9:17 PM Alexander Mikhalitsyn > <aleksandr.mikhalitsyn@canonical.com> wrote: > > > > 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. > Hi Eric, > Can you describe what side effects this patch has ? > > I think it could break some applications, who might not be able to > recover from MSG_CTRUNC in this case. > This should be documented, in order to avoid a future revert. Yes, it can break applications but only those who use SO_PASSSEC and not properly check MSG_CTRUNC. According to the recv(2) man: MSG_CTRUNC indicates that some control data was discarded due to lack of space in the buffer for ancillary data. So, there is no specification about a particular SCM type. It seems more correct to handle SCM_SECURITY the same way as SCM_RIGHTS / SCM_CREDENTIALS. > > In any case, net-next is currently closed. Oh, I'm sorry. Kind regards, Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-02-28 15:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-02-26 20:17 [PATCH net-next] scm: fix MSG_CTRUNC setting condition for SO_PASSSEC Alexander Mikhalitsyn 2023-02-27 9:47 ` Leon Romanovsky 2023-02-27 9:55 ` Aleksandr Mikhalitsyn 2023-02-27 18:32 ` Leon Romanovsky 2023-02-28 10:06 ` Aleksandr Mikhalitsyn 2023-02-28 14:45 ` Leon Romanovsky 2023-02-28 15:10 ` Aleksandr Mikhalitsyn 2023-02-27 10:01 ` Eric Dumazet 2023-02-27 10:21 ` Aleksandr Mikhalitsyn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).