wine fails to start with seccomp updates for v5.9-rc1

wine fails to start with seccomp updates for v5.9-rc1

[Alex Xu (Hello71)]

Hi,

On Linus' master, wine fails to start with the following error:

wine client error:0: write: Bad file descriptor

This issue is not present on 5.8. It appears to be caused by failure to 
write to a pipe FD received via SCM_RIGHTS. Therefore, I tried reverting 
9ecc6ea491f0, which resolved the issue.

Thanks,
Alex.

Re: wine fails to start with seccomp updates for v5.9-rc1

[Linus Torvalds]

On Fri, Aug 7, 2020 at 8:19 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>
> On Linus' master, wine fails to start with the following error:
>
> wine client error:0: write: Bad file descriptor
>
> This issue is not present on 5.8. It appears to be caused by failure to
> write to a pipe FD received via SCM_RIGHTS. Therefore, I tried reverting
> 9ecc6ea491f0, which resolved the issue.

Would you mind trying to bisect exactly where it happens?

I don't think any of the commits in that pull are supposed to change
semantics, and while reverting the whole merge shows that yes, that's
what brought in the problems, it would be good to pinpoint just which
change breaks so that we can fix just that thing.

Kees, ideas?

                 Linus

Re: wine fails to start with seccomp updates for v5.9-rc1

[Thadeu Lima de Souza Cascardo]

On Fri, Aug 07, 2020 at 08:48:46AM -0700, Linus Torvalds wrote:
> On Fri, Aug 7, 2020 at 8:19 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> >
> > On Linus' master, wine fails to start with the following error:
> >
> > wine client error:0: write: Bad file descriptor
> >
> > This issue is not present on 5.8. It appears to be caused by failure to
> > write to a pipe FD received via SCM_RIGHTS. Therefore, I tried reverting
> > 9ecc6ea491f0, which resolved the issue.
> 
> Would you mind trying to bisect exactly where it happens?
> 

This report [1] seemed related and pointed out at c0029de50982 ("net/scm:
Regularize compat handling of scm_detach_fds()"). The use of CMSG_USER_DATA
instead of CMSG_COMPAT_DATA seems fishy.

Alex, can you try applying the patch below?

Cascardo.

[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216156.html

> I don't think any of the commits in that pull are supposed to change
> semantics, and while reverting the whole merge shows that yes, that's
> what brought in the problems, it would be good to pinpoint just which
> change breaks so that we can fix just that thing.
> 
> Kees, ideas?
> 
>                  Linus

---
diff --git a/net/compat.c b/net/compat.c
index 703acb51c698..95ce707a30a3 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -294,7 +294,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm)
 		(struct compat_cmsghdr __user *)msg->msg_control;
 	unsigned int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0;
 	int fdmax = min_t(int, scm_max_fds_compat(msg), scm->fp->count);
-	int __user *cmsg_data = CMSG_USER_DATA(cm);
+	int __user *cmsg_data = CMSG_COMPAT_DATA(cm);
 	int err = 0, i;
 
 	for (i = 0; i < fdmax; i++) {

Re: wine fails to start with seccomp updates for v5.9-rc1

[Kees Cook]

On Fri, Aug 07, 2020 at 02:36:09PM -0300, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Aug 07, 2020 at 08:48:46AM -0700, Linus Torvalds wrote:
> > On Fri, Aug 7, 2020 at 8:19 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
> > >
> > > On Linus' master, wine fails to start with the following error:
> > >
> > > wine client error:0: write: Bad file descriptor
> > >
> > > This issue is not present on 5.8. It appears to be caused by failure to
> > > write to a pipe FD received via SCM_RIGHTS. Therefore, I tried reverting
> > > 9ecc6ea491f0, which resolved the issue.
> > 
> > Would you mind trying to bisect exactly where it happens?
> > 
> 
> This report [1] seemed related and pointed out at c0029de50982 ("net/scm:
> Regularize compat handling of scm_detach_fds()"). The use of CMSG_USER_DATA
> instead of CMSG_COMPAT_DATA seems fishy.

Argh; yes. Thank you for finding that! That's what I get for trying to
regularize the compat path. :(

> Alex, can you try applying the patch below?
> 
> Cascardo.
> 
> [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216156.html
> 
> > I don't think any of the commits in that pull are supposed to change
> > semantics, and while reverting the whole merge shows that yes, that's
> > what brought in the problems, it would be good to pinpoint just which
> > change breaks so that we can fix just that thing.
> > 
> > Kees, ideas?
> > 
> >                  Linus
> 
> ---
> diff --git a/net/compat.c b/net/compat.c
> index 703acb51c698..95ce707a30a3 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -294,7 +294,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm)
>  		(struct compat_cmsghdr __user *)msg->msg_control;
>  	unsigned int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0;
>  	int fdmax = min_t(int, scm_max_fds_compat(msg), scm->fp->count);
> -	int __user *cmsg_data = CMSG_USER_DATA(cm);
> +	int __user *cmsg_data = CMSG_COMPAT_DATA(cm);
>  	int err = 0, i;
>  
>  	for (i = 0; i < fdmax; i++) {

That almost certainly will fix the problem.

-- 
Kees Cook

Re: wine fails to start with seccomp updates for v5.9-rc1

[Alex Xu (Hello71)]

Excerpts from Thadeu Lima de Souza Cascardo's message of August 7, 2020 1:36 pm:
> On Fri, Aug 07, 2020 at 08:48:46AM -0700, Linus Torvalds wrote:
>> On Fri, Aug 7, 2020 at 8:19 AM Alex Xu (Hello71) <alex_y_xu@yahoo.ca> wrote:
>> >
>> > On Linus' master, wine fails to start with the following error:
>> >
>> > wine client error:0: write: Bad file descriptor
>> >
>> > This issue is not present on 5.8. It appears to be caused by failure to
>> > write to a pipe FD received via SCM_RIGHTS. Therefore, I tried reverting
>> > 9ecc6ea491f0, which resolved the issue.
>> 
>> Would you mind trying to bisect exactly where it happens?
>> 
> 
> This report [1] seemed related and pointed out at c0029de50982 ("net/scm:
> Regularize compat handling of scm_detach_fds()"). The use of CMSG_USER_DATA
> instead of CMSG_COMPAT_DATA seems fishy.
> 
> Alex, can you try applying the patch below?
> 
> Cascardo.
> 
> [1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-August/216156.html
> 
>> I don't think any of the commits in that pull are supposed to change
>> semantics, and while reverting the whole merge shows that yes, that's
>> what brought in the problems, it would be good to pinpoint just which
>> change breaks so that we can fix just that thing.
>> 
>> Kees, ideas?
>> 
>>                  Linus
> 
> ---
> diff --git a/net/compat.c b/net/compat.c
> index 703acb51c698..95ce707a30a3 100644
> --- a/net/compat.c
> +++ b/net/compat.c
> @@ -294,7 +294,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm)
>  		(struct compat_cmsghdr __user *)msg->msg_control;
>  	unsigned int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0;
>  	int fdmax = min_t(int, scm_max_fds_compat(msg), scm->fp->count);
> -	int __user *cmsg_data = CMSG_USER_DATA(cm);
> +	int __user *cmsg_data = CMSG_COMPAT_DATA(cm);
>  	int err = 0, i;
>  
>  	for (i = 0; i < fdmax; i++) {
> 

Yes, this seems to work.

Tested-by: Alex Xu (Hello71) <alex_y_xu@yahoo.ca>

Thanks!