Received: by 2002:a25:e74b:0:0:0:0:0 with SMTP id e72csp1578659ybh; Thu, 16 Jul 2020 16:44:42 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxjHOwHwyW8QDPYFbmrja/1/LRHJEPM61p9//sI8uDbaTBxjfoyD1M/3KMHdJz+agjAOCVK X-Received: by 2002:a17:906:dbed:: with SMTP id yd13mr5897507ejb.419.1594943082744; Thu, 16 Jul 2020 16:44:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1594943082; cv=none; d=google.com; s=arc-20160816; b=UhNOYZsbahSU8Og/KG45BjynAsJ5w+68OFzndqWHbSOHWIQme/x6l5sfd5hvBErQFu GtWAtLk3Jym8SwOuOAOCpea4UM/DqektUGxeUak+f8gri+8jLeFMNZjSqoycUribL3Us tyX4y1OZzjNjdp6rhtH5TItHKWn+smOzrLO+iBn0iEuNFYzOW3I8Rk+Nc+yDY/yuDUrG fX4BP3mHgzKWWYPqm2rVJarTJcdPxosr6fH7DrxWLGg3POJPvWUk4rWfRk5pbjiRc+pY 5WG2TZPXks1yNJBCEZOyUjVcaD5yb9SHYolFQ5Qn512OZ0rS/XQiDSQk+QPnIQYTGh+f 20BQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:mime-version:message-id:references :in-reply-to:subject:cc:date:to:from; bh=r4hNfBbkQCF9FDbIP9eoNKhPavs2kiPpQD/dpL9n3Cg=; b=Y8asRwYU6MhHUmH3MtKVECPkcq+f1eH2ITsw1XMRlJeSi4iCTI9LO7hClgLbUk2/nr ETrJsSJ45B6Z1YE9Hd/b8JVcapQahh2yRhxQJ2rzrRMWCFZDLu1kBDIk8eczeOdFuYXu WlOy5HTDZt2uK/gezwNdsparBcAtxnR1a1133M1uGItQY/8okWsHiYgqE2OYjKKRU4A1 MEAZYdKSPCPGf3nAjuy4yodd2fSYHLRWLQue4kzCKQgOjUzqL6tvzTZ7SnIJh1HRgS3l swu+nkzWSndVQqfEHqWZHxo1wdprCHhR72qH2GXZKzs5PQb1j7n47CqJLK/SyQe9jQuk VTnA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id cx20si3756125edb.568.2020.07.16.16.43.48; Thu, 16 Jul 2020 16:44:42 -0700 (PDT) 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; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-nfs-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726166AbgGPXnr (ORCPT + 99 others); Thu, 16 Jul 2020 19:43:47 -0400 Received: from mx2.suse.de ([195.135.220.15]:51462 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726125AbgGPXnr (ORCPT ); Thu, 16 Jul 2020 19:43:47 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 56D08AB89; Thu, 16 Jul 2020 23:43:49 +0000 (UTC) From: NeilBrown To: "J. Bruce Fields" Date: Fri, 17 Jul 2020 09:43:40 +1000 Cc: Jeff Layton , linux-nfs@vger.kernel.org Subject: Re: nfs4_show_superblock considered harmful :-) In-Reply-To: <20200716171958.GB18568@fieldses.org> References: <871rn38suc.fsf@notabene.neil.brown.name> <20200529220608.GA22758@fieldses.org> <87a71n7dek.fsf@notabene.neil.brown.name> <20200715185456.GE15543@fieldses.org> <20200716171958.GB18568@fieldses.org> Message-ID: <87tuy7vxeb.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Thu, Jul 16 2020, J. Bruce Fields wrote: > On Wed, Jul 15, 2020 at 02:54:56PM -0400, J. Bruce Fields wrote: >> commit 4eef57aa4fc0 >> Author: J. Bruce Fields >> Date: Wed Jul 15 13:31:36 2020 -0400 >>=20 >> nfsd4: fix NULL dereference in nfsd/clients display code >>=20=20=20=20=20 >> Reported-by: NeilBrown >> Signed-off-by: J. Bruce Fields >>=20 >> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c >> index ab5c8857ae5a..08b8376c74d7 100644 >> --- a/fs/nfsd/nfs4state.c >> +++ b/fs/nfsd/nfs4state.c >> @@ -507,6 +507,16 @@ find_any_file(struct nfs4_file *f) >> return ret; >> } >>=20=20 >> +static struct nfsd_file *find_deleg_file(struct nfs4_file *f) >> +{ >> + struct nfsd_file *ret; >> + >> + spin_lock(&f->fi_lock); >> + ret =3D nfsd_file_get(f->fi_deleg_file); >> + spin_unlock(&f->fi_lock); >> + return ret; >> +} >> + > ... >> @@ -2513,7 +2527,9 @@ static int nfs4_show_deleg(struct seq_file *s, str= uct nfs4_stid *st) >>=20=20 >> ds =3D delegstateid(st); >> nf =3D st->sc_file; >> - file =3D nf->fi_deleg_file; >> + file =3D find_deleg_file(nf); >> + if (!file) >> + return 0; >>=20=20 >> seq_printf(s, "- "); >> nfs4_show_stateid(s, &st->sc_stateid); > > Oops, I added a "get" without a corresponding "put". > > --b. > > commit 8d2edfe45286 > Author: J. Bruce Fields > Date: Wed Jul 15 13:31:36 2020 -0400 > > nfsd4: fix NULL dereference in nfsd/clients display code >=20=20=20=20=20 > We hold the cl_lock here, and that's enough to keep stateid's from go= ing > away, but it's not enough to prevent the files they point to from goi= ng > away. Take fi_lock and a reference and check for NULL, as we do in > other code. >=20=20=20=20=20 > Reported-by: NeilBrown > Fixes: 78599c42ae3c ("nfsd4: add file to display list of client's ope= ns") > Signed-off-by: J. Bruce Fields > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index ab5c8857ae5a..9d45117c8c18 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -507,6 +507,16 @@ find_any_file(struct nfs4_file *f) > return ret; > } >=20=20 > +static struct nfsd_file *find_deleg_file(struct nfs4_file *f) > +{ > + struct nfsd_file *ret; > + > + spin_lock(&f->fi_lock); > + ret =3D nfsd_file_get(f->fi_deleg_file); A test on f->fi_deleg_file being non-NULL would make this look safer. It would also make the subsequent test on the return value appear sane. Thanks, NeilBrown > + spin_unlock(&f->fi_lock); > + return ret; > +} > + > static atomic_long_t num_delegations; > unsigned long max_delegations; >=20=20 > @@ -2444,6 +2454,8 @@ static int nfs4_show_open(struct seq_file *s, struc= t nfs4_stid *st) > oo =3D ols->st_stateowner; > nf =3D st->sc_file; > file =3D find_any_file(nf); > + if (!file) > + return 0; >=20=20 > seq_printf(s, "- "); > nfs4_show_stateid(s, &st->sc_stateid); > @@ -2481,6 +2493,8 @@ static int nfs4_show_lock(struct seq_file *s, struc= t nfs4_stid *st) > oo =3D ols->st_stateowner; > nf =3D st->sc_file; > file =3D find_any_file(nf); > + if (!file) > + return 0; >=20=20 > seq_printf(s, "- "); > nfs4_show_stateid(s, &st->sc_stateid); > @@ -2513,7 +2527,9 @@ static int nfs4_show_deleg(struct seq_file *s, stru= ct nfs4_stid *st) >=20=20 > ds =3D delegstateid(st); > nf =3D st->sc_file; > - file =3D nf->fi_deleg_file; > + file =3D find_deleg_file(nf); > + if (!file) > + return 0; >=20=20 > seq_printf(s, "- "); > nfs4_show_stateid(s, &st->sc_stateid); > @@ -2529,6 +2545,7 @@ static int nfs4_show_deleg(struct seq_file *s, stru= ct nfs4_stid *st) > seq_printf(s, ", "); > nfs4_show_fname(s, file); > seq_printf(s, " }\n"); > + nfsd_file_put(file); >=20=20 > return 0; > } --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEG8Yp69OQ2HB7X0l6Oeye3VZigbkFAl8Q5iwACgkQOeye3VZi gbkgVw/7B6AED/Dyay+AmC5fSq9d1f65zxJWQkFAn1wQ6fTXGIMhGFgqWH2JSY9y 7v38meJfAUcvhjclBNX3nngJaNLeEBFo4YlmMJkuHR4aIFDAIN6BabuJX2dUrvsW D4PBPX6mfPLywkMuWFoxGANkP8BGXUMHN7gU/9+rvHdKEupnocSEiay9+dyBvC3H le0JDW63hHiWHsEWWu1LnQ28JfaVoVUhy7gv4n7/eHiRaP4rZTbS+ArCskAeEC5N fJ1ZT+h3AxiyPlqiliNLSkj3XBKw3XYlyQFpx3FIEoqVFZ3Nav6MEsu/4eFbmkRm jgoGC3DtFOAS/OMOdBzSzsEmXLht+D+KUBZLS66MBHkrBqzLcpkxKgI32NPQUu/n UyIetlGfHoxM0w8S4hRQqSf/y9qg8n7lML/bHwqJW7E0wKWov1OTy0YL9B929duS HS5zmuWSNWTsZyyjY9X5HajyK2AsObACBdxTV59Rtc4Hokt8oaCVdPIdKnBAkKBL +1nfB9a6u7ZaV/yxvbJKkodcwhRxCLt63TT0B8CWpbUuk3oiOCkI0gfhm8WXaQhR FuewuTNHDwXFDDBjYsdFM0jdkHEd7BBfoCbQwodv9qFgzst0xUC4Y3hgRCFJsZRv urHnXluv3cXPPmv+tUVQ1ayaZG8Q/YQ3OIrArbXoPwjracBtdRI= =jfBQ -----END PGP SIGNATURE----- --=-=-=--