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:     "Russell King (Oracle)" <linux@armlinux.org.uk>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: 5.13 NFSD: completely broken?
Thread-Topic: 5.13 NFSD: completely broken?
Thread-Index: AQHXbcgaFJnLmMlpmEOPdhWd8pFOCasstRGAgAAC7ACAAAZyAA==
Date:   Wed, 30 Jun 2021 16:28:47 +0000
Message-ID: <EEA82A83-433C-49C2-BD70-BBAE98961705@oracle.com>
References: <20210630155325.GD22278@shell.armlinux.org.uk>
 <1BF34FA4-EC1A-405D-AA8B-217BF94DA219@oracle.com>
 <20210630160542.GD20229@fieldses.org>
In-Reply-To: <20210630160542.GD20229@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?Mu+g0uxqWSdzqdGh+0NY74hLRAGgWJmbk0R+RbxE9icw1+oKodlW9cXUkI5o?=
 =?us-ascii?Q?oGNFf/pfHD3KUWZ5lJ5UmLty72SEdSjR2Kd+zuZdUsBDszGGvWccZQn1w8H5?=
 =?us-ascii?Q?b4eri7cR+fusMHkEVvlO1+orxPyVMk1rh8mBGVT5Ae+tmboegCefwtQW7VCP?=
 =?us-ascii?Q?VnYqValtCS1mCKPlFxXwailOatfweE0W82ef9AnHTMlxGms9OMGNv8tkRllG?=
 =?us-ascii?Q?I042PC1iizIMoDZFE0aGURi6wlCb828Wtt+GQtffzYL1rIFzKkLCAJmrW31A?=
 =?us-ascii?Q?hZtUOlQsT1yoPttFtSFthq0N7Q+ZFGmF7o2LeBhJPCMugR45wCuDDALaQtix?=
 =?us-ascii?Q?e9ThiuUZ481fXymdxPOocV/szb3tRx6BOX61XpvuLAKi/HpQONha9jL0racI?=
 =?us-ascii?Q?gS2+DhOJY7VuMKk0p6ZMMR4fDPTf5Y9yq7cwVaiB2u/h+wEZee+LrBzM8QYt?=
 =?us-ascii?Q?Q2BpfF+CgIhL3h7URQsDJz3+OCV5xHx0R39USAvuihRkTbLLqS2Sf1n3MIuT?=
 =?us-ascii?Q?MDZ7OIl2rBhjR9uOJPuQbJ4yVwkNDPzqyZVdAFl7rAw6jWJeSUsuoWJijIlM?=
 =?us-ascii?Q?7tCKnnE61XPy9JVskIGikSSctY4qq7JUPDHhuq7F/7AjXc+yTvKQGjsBA3xM?=
 =?us-ascii?Q?+rycbO3REtY4ZipbVbvcw3TFioB51zMHYS2vWlMXb8Ik/L7jumeH7gYA+tSM?=
 =?us-ascii?Q?vkJmWI8w+Pc+ChYxVYToAbczKyLltkHsiBl+E5B+5c5W7cLNfGesCbivsqzf?=
 =?us-ascii?Q?e/NZBQQ2xB8S08P/lR+Bu7MgfhLMZIRkhpBMILrsmKtjt6uI2N4hJQK2IW1H?=
 =?us-ascii?Q?JDtrlr+TNH68uh2kf0u9sj613rGlblBrP/S5MwS7GTmzbVNr6/0btVSSGJ/p?=
 =?us-ascii?Q?3aA09VtkNV4flE8cNBr9o0yV5/99/iMxxYLtlSysi9ckoDxBApDKdfSrwN/A?=
 =?us-ascii?Q?P9uA8+D8WtZJvm5HOgYgfv275u5Zaew7BG3dY/pLSA8HVvQVbLeqcxPpdq41?=
 =?us-ascii?Q?VqSmwqK8DTcnjoksC2tds9+4UGG+zoQ/XF1GwBg14O7B2mUg1J44Ijyi1dIo?=
 =?us-ascii?Q?4DBzJSRIP3ak7vHQYTsseZUGOafppRs12BY07n5l7I+HxJjkiJ0L/Bxe1Lzt?=
 =?us-ascii?Q?j1GdnXPbMInLyysHK7oy2A6idTkazcerczcbL3mOHnB16V7wouLvl+y87p3r?=
 =?us-ascii?Q?QJO27ASL8m4M2CZ1wkkwUkphg+rFT1adbkktOkZV2rjb2vRvZtKx82kkk7cG?=
 =?us-ascii?Q?oZLRIhbdsEn4IqO62EzM5kalZyWQQa+A+jeA8vImZf91RehcfC84s7I7Bs+/?=
 =?us-ascii?Q?rd+qOh3Uhyu5Hyt/4f847qva?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="us-ascii"
Content-ID: <94321C775CC0074F84740E5AD7A97560@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: b73e0b3a-b7e3-42f6-96a3-08d93be42337
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Jun 2021 16:28:47.7281
 (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: dgidqVwUD/IXzk+BNug96sHphd0UpWvq0T/KTDhBzrBSFWkno96ln+yqPItDca1jP2rQHcOHKvqiM1OvBRzusg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB4275
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10031 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0
 mlxlogscore=999 phishscore=0 spamscore=0 malwarescore=0 mlxscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2104190000 definitions=main-2106300094
X-Proofpoint-GUID: tucwZaPjlADWD1Ikn_bNKCw6M4lRfZR8
X-Proofpoint-ORIG-GUID: tucwZaPjlADWD1Ikn_bNKCw6M4lRfZR8
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jun 30, 2021, at 12:05 PM, Bruce Fields <bfields@fieldses.org> wrote:
>=20
> On Wed, Jun 30, 2021 at 03:55:16PM +0000, Chuck Lever III wrote:
>>> On Jun 30, 2021, at 11:53 AM, Russell King (Oracle) <linux@armlinux.org=
.uk> wrote:
>>> Has 5.13 been tested with nfsv3 ?
>>>=20
>>> Any ideas what's going on?
>>=20
>> Yes, fortunately!
>>=20
>> Have a look at commit 66d9282523b for a one-liner fix, it should apply
>> cleanly to v5.13.
>=20
> So was this a freak event or do we need some process change?

I would say that it is "Somewhat freaky".

In the future we expect there to be more consumers of the
bulk page allocator, so this kind of thing should become
less likely over time.


> Looks like the problem commit went in 3 days before 5.13.  It was an mm
> commit so they're probably not set up to do nfsd testing, but nfsd is
> the only user of that function, right?

NFSD is one of two consumers of that function. The other
is __page_pool_alloc_pages_slow().

The author of __page_pool_alloc_pages_slow() was cc'd on
the broken fix, but I was not. I was cc'd on earlier
traffic regarding the new API because I instigated its
inclusion into the mainline kernel on behalf of NFSD.

Mel does indeed have the ability to test NFSD, and has
been careful in the past about testing before committing.


> It says it fixed a bug that could result in writing past the end of an
> array, so made sense that it was urgent.

No, a prior commit, b08e50dd6448 ("mm/page_alloc:
__alloc_pages_bulk(): do bounds check before accessing
array"), fixed that particular bug. The broken patch
fixed a page leak in the same use case.

I tested the new API with NFSD extensively, and did not
notice page leaks or array overruns (although, rq_pages
has an extra element at the end for other reasons).

NFSD does feed full arrays to alloc_pages_bulk_array()
quite often with RDMA, so I think I would have noticed
something. With TCP I think there is at least one missing
page per call, so the common case wouldn't have triggered
this bug at all.

__page_pool_alloc_pages_slow() always passes in an empty
array, so it also would never have hit this bug.

The problem was discovered by static analysis, not a
crash report. IMO the fix should have waited for testing
and review by the NFS community. This close to a final
release, the fixes for the array overrun and page leak
could have gone into v5.13.1. I hope automation will
take care of all of this very soon.


--
Chuck Lever