Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Bruce Fields <bfields@fieldses.org>
CC:     Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] nfsd: fix NULL dereference in nfs3svc_encode_getaclres
Thread-Topic: [PATCH] nfsd: fix NULL dereference in nfs3svc_encode_getaclres
Thread-Index: AQHXb1g9rry/qWwViESBGsAmTSv7g6swC2EA
Date:   Fri, 2 Jul 2021 19:04:09 +0000
Message-ID: <19495FB8-43F1-465E-AABE-E0049F578D22@oracle.com>
References: <20210702153751.GA29685@fieldses.org>
In-Reply-To: <20210702153751.GA29685@fieldses.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?oCQUH+J1t+ph2eRgB0Z2koomIjOPj4N3yaAFGGMx4jYk/QdHOtSxkewdNykG?=
 =?us-ascii?Q?zVRHy/zR+GF3vVyeT2BtGGM9U3OubX5osC1w9BfR0+ZHUCo9GuztOhdlFh6f?=
 =?us-ascii?Q?D3sEI0EBTlka7uBZJAtswT89mI8GcqT0qiguHm/x7TrJjTJvGksJwDN+e7sg?=
 =?us-ascii?Q?vTu6Fxq2wulNCRWNSzo/FGFGuhcKz35gMjUTd3k7y0w6o/qyiV53u7toqY9f?=
 =?us-ascii?Q?ALitdQXrzhrsRU2WXer9LoVspRxMOl/vAnRs1bXtIMhTiClATyMd5z0iRnp4?=
 =?us-ascii?Q?0dTg2+BerCPDpU10gAJPEcahiyW1pUf4TJlv+lhLjiLQ3g3jUx3UqMvidvZd?=
 =?us-ascii?Q?EjKZEd5JnZv5Ao0CTZc4I4/BbQlrEn7KMSJpLpspiI320xl3eihQqkk5zHaK?=
 =?us-ascii?Q?PkHpjLoeZpWi3YA2P6uyJeg73kPZ2GQ/gUK8hlTLmZIxaIGOExnoQVmN6296?=
 =?us-ascii?Q?13gqTZk4OJ8n06iCHZQq0x5O0AafvDBUkGUikhNNHbeVKIQDW4eudvaNKHYZ?=
 =?us-ascii?Q?fAaStsaVP3LQP7006UQNqMaCx63xfkvOJaBNK25xykcPXzDAVAMQBgf2LoO0?=
 =?us-ascii?Q?NYCMz24WPq5ST5d13WtzaZj/VfE/N8d9pT7HewxcIthNX2GgKM8+n0qTrROI?=
 =?us-ascii?Q?4Ktyp5drg+z3P3jFm2F4zEkWsOOnZ88XLC4c4HapaSHx/qkiiZxbR4n8pDfL?=
 =?us-ascii?Q?wYXAG9OThR0MqF9UgwyW8yD5oU0gffRBXWF+SpFKEITWHC053jrtO6sMGJpJ?=
 =?us-ascii?Q?us188oozprutxxF1x+qsBXZgrz5mhSGlxpi6ELylr7Nfm/Xe7AuB0hJj40W4?=
 =?us-ascii?Q?QeS6+zyfBORSQinbUbnHjonDr7nTUoPkjacU6crvFgmwo7BoFqdUQubJhiIU?=
 =?us-ascii?Q?Q8uteGfNRkn4B1T/7MXFTL0AwpCPXhrU3IaFNKPN6ENL3BdnP+yGivwkv746?=
 =?us-ascii?Q?zCBYlR+goyLWy4VCA6fifbWa0b8xtheWrfhvt0VCN1vwdCXOxDDpoOSkpdM2?=
 =?us-ascii?Q?ll6WlRT1x/taCQrZRSkQnJ5CUq6U18KxRC6LCfjyLC8qh7ithNHgfiCOG2US?=
 =?us-ascii?Q?W5wK4swiNRYRgtubl6O8x3tnH+d662QiRuqNpIGXt02h/s+tHcz95bttK21Q?=
 =?us-ascii?Q?WByMV9kND5C+a9sDSMhi2av8AczLqsaR1M2j/pmPuiE0YFtF/VDX2YD2RHK3?=
 =?us-ascii?Q?rsRqe3c/EoBhFnlQMC6xDXjQl/zYHEgTKOc+EHj9gWr9F/25GAa2z3iBkxD6?=
 =?us-ascii?Q?einYeq+DS8sM+7xUj9yFBsY7a+GiCd0X/IG+6Zd/oPaUVX4gEFC2oi14u8x2?=
 =?us-ascii?Q?Cz2FaLeOTKhG27waJF+0EM16?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6CB662F62E5B4145B9E710DF5540B3F4@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR10MB4688.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e08513fe-1f9e-471c-978d-08d93d8c2bf9
X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jul 2021 19:04:09.1621
 (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: HXMoq2cmiVCu7UTeFgWewIdH3Gmd1VxRGmJNbgfuwfPGgm+peM8CryotAQKcpq49XlqYHTobdgLCcxoDTyqaCQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB4527
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10033 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 spamscore=0 adultscore=0
 malwarescore=0 mlxlogscore=999 suspectscore=0 phishscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2107020098
X-Proofpoint-GUID: uJQdcUKavc7uKNWwaHDi5Je3yX1aEjii
X-Proofpoint-ORIG-GUID: uJQdcUKavc7uKNWwaHDi5Je3yX1aEjii
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jul 2, 2021, at 11:37 AM, J. Bruce Fields <bfields@fieldses.org> wrote=
:
>=20
> From: "J. Bruce Fields" <bfields@redhat.com>
>=20
> In error cases the dentry may be NULL.
>=20
> Before 20798dfe249a, the encoder also checked dentry and
> d_really_is_positive(dentry), but that looks like overkill to me--zero
> status should be enough to guarantee a positive dentry.
>=20
> This isn't the first time we've seen an error-case NULL dereference
> hidden in the initialization of a local variable in an xdr encoder.  But
> I went back through the other recent rewrites and didn't spot any
> similar bugs.
>=20
> Reported-by: JianHong Yin <jiyin@redhat.com>
> Fixes: 20798dfe249a ("NFSD: Update the NFSv3 GETACL result encoder...")
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>

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


> ---
> fs/nfsd/nfs3acl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>=20
> diff --git a/fs/nfsd/nfs3acl.c b/fs/nfsd/nfs3acl.c
> index a1591feeea22..5dfe7644a517 100644
> --- a/fs/nfsd/nfs3acl.c
> +++ b/fs/nfsd/nfs3acl.c
> @@ -172,7 +172,7 @@ static int nfs3svc_encode_getaclres(struct svc_rqst *=
rqstp, __be32 *p)
> 	struct nfsd3_getaclres *resp =3D rqstp->rq_resp;
> 	struct dentry *dentry =3D resp->fh.fh_dentry;
> 	struct kvec *head =3D rqstp->rq_res.head;
> -	struct inode *inode =3D d_inode(dentry);
> +	struct inode *inode;
> 	unsigned int base;
> 	int n;
> 	int w;
> @@ -181,6 +181,7 @@ static int nfs3svc_encode_getaclres(struct svc_rqst *=
rqstp, __be32 *p)
> 		return 0;
> 	switch (resp->status) {
> 	case nfs_ok:
> +		inode =3D d_inode(dentry);
> 		if (!svcxdr_encode_post_op_attr(rqstp, xdr, &resp->fh))
> 			return 0;
> 		if (xdr_stream_encode_u32(xdr, resp->mask) < 0)
> --=20
> 2.31.1
>=20

--
Chuck Lever