Received-SPF: pass (google.com: domain of linux-nfs+bounces-3491-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 30 May 2024 11:16:33 -0400
From: Chuck Lever <chuck.lever@oracle.com>
To: Benjamin Coddington <bcodding@redhat.com>
Cc: Chen Hanxiao <chenhx.fnst@fujitsu.com>,
        Trond Myklebust <trondmy@kernel.org>, Anna Schumaker <anna@kernel.org>,
        Jeff Layton <jlayton@kernel.org>, Neil Brown <neilb@suse.de>,
        Olga Kornievskaia <kolga@netapp.com>, Dai Ngo <Dai.Ngo@oracle.com>,
        Tom Talpey <tom@talpey.com>, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] SUNRPC: return proper error from gss_wrap_req_priv
Message-ID: <ZliYUa8hNoHgLIkB@tissot.1015granger.net>
References: <20240523084716.524-1-chenhx.fnst@fujitsu.com>
 <E6007B79-DBB0-413A-9C46-6860AA25782D@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <E6007B79-DBB0-413A-9C46-6860AA25782D@redhat.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?Ha2EMzSWtN+ed+mF5q/9Mzob91MyuJhDx0cVVf1cYEc4EaaU4bchGzLg81yY?=
 =?us-ascii?Q?gJfiEWgvHy0ppNjV3jihLhtZTVe70aHJ2jXI/NhdtYF7RA/Nexw6qzNcDWXU?=
 =?us-ascii?Q?dxS1FOTj784kbweBKotrMoMFZRPxrYi+BPeni2cD5Ruczjs2vw7Bqa9rN17v?=
 =?us-ascii?Q?F2BXofvzbZFr8sF2d2LjW08wGcfsjIpbyilbfLvPEZOfNdw+K46hog/0ouGB?=
 =?us-ascii?Q?Yde9PzzX8QuAI7kZtXq3ENza+tXTYn49dzRDoczQoxwucwOcU6avqFqD/uvp?=
 =?us-ascii?Q?oeBfOy76+KuJXjSpJawnJTWPd6BbnyoF2QDOrlGwlDN05+blBBq69v/O8Jou?=
 =?us-ascii?Q?0UxPCetfmEc3y8v1FF8Me4ZQOOZVMq3N7fW/Rferxkqd9TavkekdPr/N86e8?=
 =?us-ascii?Q?raKW/pKJ69g8Scb3YKha1y3KXdwLzdJvDJuZJMk5u02dXrCrqhMMBVlEBoP6?=
 =?us-ascii?Q?ZXQIKAS+gHIlq81i2/Jvoz4d3LFcQlm7UCbdi+EPtUIjDx5lC6r48eXowW9t?=
 =?us-ascii?Q?XeG0jCtA/B4qnfjfBeGxLPFMhcKxLKx3+KR4Pw7jLTLqi9VuoHOy/NtVGa3F?=
 =?us-ascii?Q?c3Ecob9Aa1UxPZMLKi1lZT8Wz9a5twftab8yag0hDl+erYPw+dowLTsXQEe6?=
 =?us-ascii?Q?SToZaKjsomTEw4GnzYXOAxEhJkBffPEgE82OW07cIsp4DBG8RIRlaiK27NqM?=
 =?us-ascii?Q?3ZosRPLKV6+AYo20MOXUcuFlezXihvAr5ZEcWuVnrK7BEdQz0fMdV6yHr1Lk?=
 =?us-ascii?Q?39x/5iQQUm4b1EE4yufkYoQQAc67WWWRcHZ4kLvPmbpK6KLO2MLG+bQ65oZI?=
 =?us-ascii?Q?BLLQBsijDnybc0LsI+PiAqc3zMBWkAeLbND3NsshHqLoYiDyZRO1p2V9eD4d?=
 =?us-ascii?Q?JUPw2sB9LtuiV4l9FjyO8DypDlt5e4pCp04oHUwjqcniXLKD/Dil43Kl3IAX?=
 =?us-ascii?Q?8O/5Eib0TqMtwGGaHCfMmSmwk0FgNyCZ9yMU0hKhAnAQ+i+nbzkFAtpBL85G?=
 =?us-ascii?Q?oO3ThZVdQ+r+htBrWMnpq3F2Z6YuderVW56tdnX9BqlPKDdZj5h00VhpP9lv?=
 =?us-ascii?Q?mSr5dx2m51cKcVj9oK4ew+IzhT+R6mINHsEkArGCMbb/2asflMhu1E6GnVgw?=
 =?us-ascii?Q?bCtZs7kK8SGdzrfheUc1IublyBwrygZ5lEjYbWbvbwQN+t2hKKehSUsYJ6S6?=
 =?us-ascii?Q?NPxK7lFhRaVYOuUX3ohthjZW5FTpEsKCgSRJTkNXsgSKAmXpEH8pQZ1ODUsA?=
 =?us-ascii?Q?vO8dedYBIf/ZcTDTwCouIBQ/3wkEQ1owjl5df7TcRdKw0lZ5LklENQPtCAM3?=
 =?us-ascii?Q?VT1R8sryoFoIlOrW6oIgXZBru/K2ElsgzpYlrHYLU0csgGlI4XWsmLDn7voa?=
 =?us-ascii?Q?ZOhS4joLVp9n+yN+Av2hLddDubydzEb/uDRzEtyaj/Z70KNIstFC4VRp2HMw?=
 =?us-ascii?Q?iL7LkDNbzJd2RZG7cf7sjmdb15scyeZ9cH1/fbKlmlhDBemPqVuUf2L9752R?=
 =?us-ascii?Q?UWSDUhO7i3t7dzkUETjzTNdUvRVOENrAJajiKlLHSZJNlq7bibEa6Q3HGJat?=
 =?us-ascii?Q?Tj94//UMGL+KWDFk34aJZ0qUgDSnxY8aXP1zwU6akvkTCmSoMpp89kmr72Gn?=
 =?us-ascii?Q?2w=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	OocndXGxRdabqPsDRp5qHxkktYD/aM4GwLqIQNJanTMAW3YhpIrYdi+pY7mj34/1sbuon6QFviMHo+RNpRyNAW495mXcOAZUA1VqUpKgm1FxkKdSr2bh5nSh8F4QFNsI8RlFzt0x9+TPvLHUG6OefSmjfQazqkH/s1epBvmZ707rGLIL1WqDGS6i1DrRbAFbw4dCWyNpsnkeaOA6M8t6MO2ZeCRRwcRpKBLEy0L8nHheP410HonoGi5ibwiCJVk+7Zf5Jf84nCFF2Zr4sOBWJ5wtZnF89l6BJY6gF7KFvhP2QvLykW1Z58jkq3o+k4rdUpDdr9v+J73IXtSvwN38kfwJ5vbf21p8oOC85Hf8mKOVQ7fvgd8Nc6F2B37AwyZ/JkAemy21M3qtubqIARrv03FGvXW4RicWmCBnGae5kqSAtD5mfgsvX3MTVdPEKvIbmHIvLzy6QWOoXRKRddhhqB/C/rLyHkdgZHa3+x9lSGouMrJiB0ucvlKd7azJ3XVx+KbpGvTLc1CR5ZyPlSWb6ybVFKEzX2jlqYmVumgjVIh1FoMt5aeCOUL5rhy8KrU8X6jcdBnBfHkAQTfH8BpJkVNT7kJaNIKSkY/5v2TqF1E=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 734c2e41-fbd7-4edc-4aeb-08dc80bb7f69
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 15:16:36.6008
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Y3I6K8oFJB7BqIzuREXQTb34ZegzG9xEf3TCcLFnjobCUBqI7KaD4bZDl7tiu2/OFlViEZ2/Bo7f1Llo0XD5WA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR10MB6096
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.12.28.16
 definitions=2024-05-30_11,2024-05-30_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 malwarescore=0 mlxscore=0
 mlxlogscore=999 phishscore=0 spamscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2405300115
X-Proofpoint-ORIG-GUID: dD-ykT6vi3GgWdTKnwbHj-W9LGAUaJV3
X-Proofpoint-GUID: dD-ykT6vi3GgWdTKnwbHj-W9LGAUaJV3

On Thu, May 30, 2024 at 09:51:02AM -0400, Benjamin Coddington wrote:
> On 23 May 2024, at 4:47, Chen Hanxiao wrote:
> 
> > don't return 0 if snd_buf->len really greater than snd_buf->buflen
> >
> > Signed-off-by: Chen Hanxiao <chenhx.fnst@fujitsu.com>
> 
> Fixes: 0c77668ddb4e ("SUNRPC: Introduce trace points in rpc_auth_gss.ko")
> Reviewed-by: Benjamin Coddington <bcodding@redhat.com>
> 
> more below ..
> 
> 
> > ---
> >  net/sunrpc/auth_gss/auth_gss.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
> > index c7af0220f82f..369310909fc9 100644
> > --- a/net/sunrpc/auth_gss/auth_gss.c
> > +++ b/net/sunrpc/auth_gss/auth_gss.c
> > @@ -1875,8 +1875,10 @@ gss_wrap_req_priv(struct rpc_cred *cred, struct gss_cl_ctx *ctx,
> >  	offset = (u8 *)p - (u8 *)snd_buf->head[0].iov_base;
> >  	maj_stat = gss_wrap(ctx->gc_gss_ctx, offset, snd_buf, inpages);
> >  	/* slack space should prevent this ever happening: */
> > -	if (unlikely(snd_buf->len > snd_buf->buflen))
> > +	if (unlikely(snd_buf->len > snd_buf->buflen)) {
> > +		status = -EIO;
> >  		goto wrap_failed;
> 
> Maybe Chuck intended to jump to bad_wrap in 0c77668ddb4e?

bad_wrap is specifically for reporting a GSS failure, so "goto
wrap_failed;" is correct.

The bug here is that the earlier alloc_enc_pages() call clobbers the
default value of @status.

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


> Interesting that
> you found this considering "slack space should prevent this ever happening".

That suggests that the slack space computation is somehow wrong,
which might be possible for one of the new enctypes..? Further
investigation is warranted.


-- 
Chuck Lever