[focal:linux-azure][(additional) PATCH 1/1] cifs: do not fail __smb_send_rqst if non-fatal signals are pending

Guilherme G. Piccoli gpiccoli at canonical.com
Tue Feb 2 18:22:41 UTC 2021


On 02/02/2021 14:39, Marcelo Henrique Cerri wrote:
> From: Ronnie Sahlberg <lsahlber at redhat.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1911438
> 
> RHBZ 1848178
> 
> The original intent of returning an error in this function
> in the patch:
>   "CIFS: Mask off signals when sending SMB packets"
> was to avoid interrupting packet send in the middle of
> sending the data (and thus breaking an SMB connection),
> but we also don't want to fail the request for non-fatal
> signals even before we have had a chance to try to
> send it (the reported problem could be reproduced e.g.
> by exiting a child process when the parent process was in
> the midst of calling futimens to update a file's timestamps).
> 
> In addition, since the signal may remain pending when we enter the
> sending loop, we may end up not sending the whole packet before
> TCP buffers become full. In this case the code returns -EINTR
> but what we need here is to return -ERESTARTSYS instead to
> allow system calls to be restarted.
> 
> Fixes: b30c74c73c78 ("CIFS: Mask off signals when sending SMB packets")
> Cc: stable at vger.kernel.org # v5.1+
> Signed-off-by: Ronnie Sahlberg <lsahlber at redhat.com>
> Reviewed-by: Pavel Shilovsky <pshilov at microsoft.com>
> Signed-off-by: Steve French <stfrench at microsoft.com>
> (cherry picked from commit 214a5ea081e77346e4963dd6d20c5539ff8b6ae6)
> Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri at canonical.com>
> ---
>  fs/cifs/transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 6d6de183915b..4ffbf8f96581 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -339,7 +339,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
>  	if (ssocket == NULL)
>  		return -EAGAIN;
>  
> -	if (signal_pending(current)) {
> +	if (fatal_signal_pending(current)) {
>  		cifs_dbg(FYI, "signal pending before send request\n");
>  		return -ERESTARTSYS;
>  	}
> @@ -431,7 +431,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst,
>  
>  	if (signal_pending(current) && (total_len != send_length)) {
>  		cifs_dbg(FYI, "signal is pending after attempt to send\n");
> -		rc = -EINTR;
> +		rc = -ERESTARTSYS;
>  	}
>  
>  	/* uncork it */
> 

Thanks Marcelo, seems a simple fix/clean cherry-pick, and was tested
using the smoke test you mentioned, so:

Acked-by: Guilherme G. Piccoli <gpiccoli at canonical.com>


Curiosity question: do we plan to have that on generic kernels?
Cheers,


Guilherme



More information about the kernel-team mailing list