> > @@ -1919,6 +1936,13 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
> > goto out;
> >
> > if (msg->msg_namelen) {
> > + err = BPF_CGROUP_RUN_PROG_UNIX_SENDMSG_LOCK(sk,
> > + msg->msg_name,
> > + &msg->msg_namelen,
> > + NULL);
> > + if (err)
> > + goto out;
> > +
> > err = unix_validate_addr(sunaddr, msg->msg_namelen);
> > if (err)
> > goto out;
>
>
> Just an FYI, I /think/ this is going to introduce a bug similar to the one I'm
> addressing in my patch here:
>
> - https://lore.kernel.org/netdev/20230921234642.1111903-2-jrife@xxxxxxxxxx/
>
> With this change, callers to sock_sendmsg() in kernel space would see their
> value of msg->msg_namelen change if they are using Unix sockets. While it's
> unclear if there are any real systems that would be impacted, it can't hurt to
> insulate callers from these kind of side-effects. I can update my my patch to
> account for possible changes to msg_namelen.
That would be great! I think it makes sense to apply the same concept to unix
sockets so insulating changes to the msg_namelen seems like the way to go.
> Also, with this patch series is it possible for AF_INET BPF hooks (connect4,
> sendmsg4, connect6, etc.) to modify the address length?
This is not yet allowed. We only allow changing the unix sockaddr length at the
moment. Maybe in the future we'd want to allow changing INET6 addr lengths
as well but currently we don't allow this.
On Tue, 26 Sept 2023 at 00:13, Jordan Rife <jrife@xxxxxxxxxx> wrote:
>
> > @@ -1919,6 +1936,13 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
> > goto out;
> >
> > if (msg->msg_namelen) {
> > + err = BPF_CGROUP_RUN_PROG_UNIX_SENDMSG_LOCK(sk,
> > + msg->msg_name,
> > + &msg->msg_namelen,
> > + NULL);
> > + if (err)
> > + goto out;
> > +
> > err = unix_validate_addr(sunaddr, msg->msg_namelen);
> > if (err)
> > goto out;
>
>
> Just an FYI, I /think/ this is going to introduce a bug similar to the one I'm
> addressing in my patch here:
>
> - https://lore.kernel.org/netdev/20230921234642.1111903-2-jrife@xxxxxxxxxx/
>
> With this change, callers to sock_sendmsg() in kernel space would see their
> value of msg->msg_namelen change if they are using Unix sockets. While it's
> unclear if there are any real systems that would be impacted, it can't hurt to
> insulate callers from these kind of side-effects. I can update my my patch to
> account for possible changes to msg_namelen.
>
> Also, with this patch series is it possible for AF_INET BPF hooks (connect4,
> sendmsg4, connect6, etc.) to modify the address length?
