Re: Patch "um: port_user: Improve error handling when port-helper is not found" has been added to the 5.17-stable tree

Re: Patch "um: port_user: Improve error handling when port-helper is not found" has been added to the 5.17-stable tree

[Glenn Washburn]

Resending this because stable@vger.kernel.org using wrong header field.
Apologize for duplicates.

On Thu, 19 May 2022 09:52:07 -0400
Sasha Levin <sashal@kernel.org> wrote:

> This is a note to let you know that I've just added the patch titled
> 
>     um: port_user: Improve error handling when port-helper is not found
> 
> to the 5.17-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      um-port_user-improve-error-handling-when-port-helper.patch
> and it can be found in the queue-5.17 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.

First, I should say that I'm not familiar with the process so I'm
likely to be wrong on any number of things. Second I'm the author of
this patch and I would like to see this included in the stable trees.
However, it appears to me that there is a problem in including just this
patch, as it depends on a previous patch which does not appear to be
applied[1].

> commit efc324ad7e7e1c92a8862bd71b2f5f8f15513304
> Author: Glenn Washburn <development@efficientek.com>
> Date:   Thu Mar 3 01:53:32 2022 -0600
> 
>     um: port_user: Improve error handling when port-helper is not found
>     
>     [ Upstream commit 3cb5a7f167c620a8b0e38b0446df2e024d2243dc ]
>     
>     Check if port-helper exists and is executable. If not, write an error
>     message to the kernel log with information to help the user diagnose the
>     issue and exit with an error. If UML_PORT_HELPER was not set, write a
>     message suggesting that the user set it. This makes it easier to understand
>     why telneting to the UML instance is failing and what can be done to fix it.
>     
>     Signed-off-by: Glenn Washburn <development@efficientek.com>
>     Signed-off-by: Richard Weinberger <richard@nod.at>
>     Signed-off-by: Sasha Levin <sashal@kernel.org>
> 
> diff --git a/arch/um/drivers/port_user.c b/arch/um/drivers/port_user.c
> index 5b5b64cb1071..133ca7bf2d91 100644
> --- a/arch/um/drivers/port_user.c
> +++ b/arch/um/drivers/port_user.c
> @@ -5,6 +5,7 @@
>  
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
>  #include <errno.h>
>  #include <termios.h>
>  #include <unistd.h>
> @@ -175,6 +176,17 @@ int port_connection(int fd, int *socket, int *pid_out)
>  	if (new < 0)
>  		return -errno;
>  
> +	err = os_access(argv[2], X_OK);
> +	if (err < 0) {
> +		printk(UM_KERN_ERR "port_connection : error accessing port-helper "
> +		       "executable at %s: %s\n", argv[2], strerror(-err));
> +		if (env == NULL)

The the afore mentioned patch that this patch depends on "env" is
declared and set. Without it, I'd expect this to fail to compile. As
such, I may be wrong in that the dependent patch was not already
included because I'd expect there to have been a compile test prior to
this patch getting to this phase.

My suspicion is that the stable trees try to not include new
functionality, which the missing patch may have been considered to have
done, and thus was not included. If its deemed undesirable to include
the missing patch, this "if" block can be removed. Although, I think
the missing patch is valuable enough to include.

The above goes for all the stable branches that this patch is set to be
included in.

Glenn

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.18-rc7&id=db8109a8bb4a4b31e7f630d7667749d62ee4a087

> +			printk(UM_KERN_ERR "Set UML_PORT_HELPER environment "
> +				"variable to path to uml-utilities port-helper "
> +				"binary\n");
> +		goto out_close;
> +	}
> +
>  	err = os_pipe(socket, 0, 0);
>  	if (err < 0)
>  		goto out_close;

Re: Patch "um: port_user: Improve error handling when port-helper is not found" has been added to the 5.17-stable tree

[Sasha Levin]

On Fri, May 20, 2022 at 02:05:15PM -0500, Glenn Washburn wrote:
>Resending this because stable@vger.kernel.org using wrong header field.
>Apologize for duplicates.
>
>On Thu, 19 May 2022 09:52:07 -0400
>Sasha Levin <sashal@kernel.org> wrote:
>
>> This is a note to let you know that I've just added the patch titled
>>
>>     um: port_user: Improve error handling when port-helper is not found
>>
>> to the 5.17-stable tree which can be found at:
>>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>>
>> The filename of the patch is:
>>      um-port_user-improve-error-handling-when-port-helper.patch
>> and it can be found in the queue-5.17 subdirectory.
>>
>> If you, or anyone else, feels it should not be added to the stable tree,
>> please let <stable@vger.kernel.org> know about it.
>
>First, I should say that I'm not familiar with the process so I'm
>likely to be wrong on any number of things. Second I'm the author of
>this patch and I would like to see this included in the stable trees.
>However, it appears to me that there is a problem in including just this
>patch, as it depends on a previous patch which does not appear to be
>applied[1].

Right, I've dropped it.

>The the afore mentioned patch that this patch depends on "env" is
>declared and set. Without it, I'd expect this to fail to compile. As
>such, I may be wrong in that the dependent patch was not already
>included because I'd expect there to have been a compile test prior to
>this patch getting to this phase.
>
>My suspicion is that the stable trees try to not include new
>functionality, which the missing patch may have been considered to have
>done, and thus was not included. If its deemed undesirable to include
>the missing patch, this "if" block can be removed. Although, I think
>the missing patch is valuable enough to include.
>
>The above goes for all the stable branches that this patch is set to be
>included in.

Could you provide a list of patches that are needed here?

-- 
Thanks,
Sasha

Re: Patch "um: port_user: Improve error handling when port-helper is not found" has been added to the 5.17-stable tree

[Glenn Washburn]

On Sun, 22 May 2022 12:34:06 -0400
Sasha Levin <sashal@kernel.org> wrote:

> On Fri, May 20, 2022 at 02:05:15PM -0500, Glenn Washburn wrote:
> >Resending this because stable@vger.kernel.org using wrong header field.
> >Apologize for duplicates.
> >
> >On Thu, 19 May 2022 09:52:07 -0400
> >Sasha Levin <sashal@kernel.org> wrote:
> >
> >> This is a note to let you know that I've just added the patch titled
> >>
> >>     um: port_user: Improve error handling when port-helper is not found
> >>
> >> to the 5.17-stable tree which can be found at:
> >>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >>
> >> The filename of the patch is:
> >>      um-port_user-improve-error-handling-when-port-helper.patch
> >> and it can be found in the queue-5.17 subdirectory.
> >>
> >> If you, or anyone else, feels it should not be added to the stable tree,
> >> please let <stable@vger.kernel.org> know about it.
> >
> >First, I should say that I'm not familiar with the process so I'm
> >likely to be wrong on any number of things. Second I'm the author of
> >this patch and I would like to see this included in the stable trees.
> >However, it appears to me that there is a problem in including just this
> >patch, as it depends on a previous patch which does not appear to be
> >applied[1].
> 
> Right, I've dropped it.
> 
> >The the afore mentioned patch that this patch depends on "env" is
> >declared and set. Without it, I'd expect this to fail to compile. As
> >such, I may be wrong in that the dependent patch was not already
> >included because I'd expect there to have been a compile test prior to
> >this patch getting to this phase.
> >
> >My suspicion is that the stable trees try to not include new
> >functionality, which the missing patch may have been considered to have
> >done, and thus was not included. If its deemed undesirable to include
> >the missing patch, this "if" block can be removed. Although, I think
> >the missing patch is valuable enough to include.
> >
> >The above goes for all the stable branches that this patch is set to be
> >included in.
> 
> Could you provide a list of patches that are needed here?

Sore. Strictly speaking, the only dependency is the patch mentioned[1].
Although, I'd suggest picking up this patch[2] that's related to
improving messaging on error conditions. I'm also unsure if the
provided links are what you're looking for in terms of how to specify
patches for you. Let me know more specifically if this doesn't suffice.

Glenn

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.18-rc7&id=db8109a8bb4a4b31e7f630d7667749d62ee4a087
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.18-rc7&id=82017457957a550d7d00dde419435dd74a890887