Received-SPF: pass (google.com: domain of linux-kernel+bounces-187888-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Gulam Mohamed <gulam.mohamed@oracle.com>
To: Christoph Hellwig <hch@lst.de>
CC: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "axboe@kernel.dk" <axboe@kernel.dk>,
        "shinichiro.kawasaki@wdc.com"
	<shinichiro.kawasaki@wdc.com>,
        "chaitanyak@nvidia.com"
	<chaitanyak@nvidia.com>
Subject: RE: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
 detach and loop open
Thread-Topic: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
 detach and loop open
Thread-Index: AQHarOjn9QOSb4ihfkuolttTouVFhbGlI/Jw
Date: Thu, 23 May 2024 18:39:10 +0000
Message-ID: 
 <IA1PR10MB7240F50A15D30D77DA7DAE8298F42@IA1PR10MB7240.namprd10.prod.outlook.com>
References: <20240521224249.7389-1-gulam.mohamed@oracle.com>
 <20240523081207.GB1086@lst.de>
In-Reply-To: <20240523081207.GB1086@lst.de>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: 
 =?us-ascii?Q?jFIFr2fsO1+i1FSkWxCl3tc3mOym61nR+rUGyNaVFCSgreBrFtxRjgNblU7j?=
 =?us-ascii?Q?rRPEXkta+3XQExayl4JesN/3zpZndZ20PDfKfQQNVrfponoTWoC2/1WQLMSU?=
 =?us-ascii?Q?Bf/+4ljePgpUYPb8qZPNqm9zXQaKuMUq8JPozSDxhe5gwvROgJe8ep9ZrV+x?=
 =?us-ascii?Q?DwcW4N0Kxa8CslBQZx0HH31qlVc9NocoqX2J5mvg9XxUVIxRBMWsApJCTqxl?=
 =?us-ascii?Q?4P1IK9/purgMODrfhnQGqcWseH1gtny4gCJcVoNi6kgIVwi0+9mzGkJn0K82?=
 =?us-ascii?Q?cK7GnBQA0ZUnUsND8SY9JUrvwLUsYe95uL/fLuRIiCSvdlq/0jBJaU3wgYmZ?=
 =?us-ascii?Q?nmPJ/Uy2aS4CKVBWmwRz649HT/aZky0FchESSIiJG9adsGrRDyeWzpYcIheu?=
 =?us-ascii?Q?/izjIyZOKQTjA/ak6QqR7dQX7HhA1wIWXHx0DyzyhAZwYhdbGqB9ldTGExW8?=
 =?us-ascii?Q?tk8qkjOmneqn4CTM3u03R372NUoXpBpMjMyHXBbsUfuKEa+vyaneriTH3/Z3?=
 =?us-ascii?Q?3+KFtqP9+kSUwucFOsC+pbUorB16NL9pFp3u6f6Fnm4s6q2/NIT/4IN6Tmte?=
 =?us-ascii?Q?mD2ZcNNQKiyRfb+mC8Hj0Yp+G0hIe1UAd/TPRCiQkulnCzPV4hDCqxE45Qpv?=
 =?us-ascii?Q?MFXpIQju/fnc8ifakw5b3xMO+onajPKlX8fJ0EFXFf1zKQMxFCmtjeQm7mry?=
 =?us-ascii?Q?kQQ2TDWlkk2hufjRQdPCZH03J+jPZDsSdQtT8DynTpeYS4dYby0gWTFmBeLt?=
 =?us-ascii?Q?cZuqWZR9vqo5/KSNLA8nyX3tRaOlJ/jr0gNma2+arK5FVFDUkHmZhzl3uLLm?=
 =?us-ascii?Q?+NA8yF00Zl9e4+Xpb9z1KlkwGUvBsPcVWkRdwin/uvw1Unjb3XK5TJhkaZif?=
 =?us-ascii?Q?ToeKP4v6e335sH87o+8LvZEDsr47S0qeNiHYKjyCf7xq07tXhx6rEM+1MnFI?=
 =?us-ascii?Q?enMvEP5v5bx/Ci3/VY1Zpe6QCn41332joQS6ElXJbH8CzDaRbdFFDx/LcwGB?=
 =?us-ascii?Q?0DqXpmrnVIrB8rx5B5a4MlmN0XWPANusQRFze8dqNss3Y8YgWBRfU7JK5Bfq?=
 =?us-ascii?Q?UITIHHj8sWx7o4bnM4gXmeKdX1/oPY9xEhM1gNTrJaxp6rKg8BmnAFm/qN+3?=
 =?us-ascii?Q?N3WWXPti5ngbxekEn6TgjT9qDbKnvLQT+9NqlqoX5uHxm5J2Fl0MS4y/NOUw?=
 =?us-ascii?Q?ZVb2sBJsX3eDWNfa8CLw8iPgejlDJcdSD6zGQ9rzKrbIavpNX14V7yoqQBPE?=
 =?us-ascii?Q?2beEprKsOS/LhaINfhNNtXnel51A7Xd502AEnVo4FbLNqgj8OskH8bkz0nTD?=
 =?us-ascii?Q?4wiBUadQKjzwB63wYoZPrL4N8kLFM1n5GWN+8yhazg5O8AiAAl8wnhq6WJdp?=
 =?us-ascii?Q?ndnoSeY8x3MrijTAvYooOwFanjbvVkul1Mhhd1jZG+OoomSIx0hYdLtrZrte?=
 =?us-ascii?Q?+Ktkl6lLkXYk4rt2opHxbHm454IH+79KMYn/TxbMEQOcDYaOcQHXw430Llnk?=
 =?us-ascii?Q?RPxH6fJt82jYkzjh9ISJVPcHaP/WE0cAnhvcW+UR6dIcI/25kZ4xGxmL6Nec?=
 =?us-ascii?Q?2xUx2mUA2PD/+dEcm6sx8azmPfqcH0wxfRiOiu9a?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	W7HuVhNXqDZsc7yIDFIf1bnnCq89RaseCcezYuvCGI6/NO2RIfzkFFLFunLVVy3r4OJuXb2/0BsES5lBgWWknzb7ytc1XovsvlkTa0ihKnZdZaxy8AEjkKO5cULBFQDNdBr5DKVEnBg8JNjP+DQhYoTzpOdstX3cOIFRfikZU80TGJt67V15oJ/EIOaHd1dqVajObk//1CD+bHRuyyo5WKnz1VhwAhKMIq49twLkFbQpsr0LUUo34/LjnT5ODaQOiHiuU3dgnOBsirVb7nLslECQoitCDHQOed1hE2BqvDaa/pnBz5U7B5qLU7+45l5j1MUnZOlu1RKZRgKieuEgy/qAuUZ5D7W+w40Pd5cr1TNrxOQfTPVqfAVgZ+vmjTy1yftldataF1m8wYkLV9FIyf/3IXuqmqCfT+UYQM+pKA+YP/eMnPgqLNY8l4Jo2OOHulXUR+SInvnbHi+/VyfCEystOEgzO4f2iJ49kMauPqvxy51EiZ1VTExJUBGaGgki8hZYha91hn/UvRHQnDSaUBgU7AFy1yZHbVV+tvHoCo3FEioUKgOw3KB00XdlAh0O8gPd3qZobC0TY/5qbw65T8HmQM+INAjrA4ylf0NEU58=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: IA1PR10MB7240.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a839b0c2-6e5a-4264-b7e8-08dc7b57a311
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 May 2024 18:39:10.7779
 (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: HL94gpioTZZjNxArZznX8W43HEFeSGK8QS4EppER+ZyRfoFQv8Exl6AH0lTrEPAfJixXRGnGz9q5716Wg3RWnRIJw5hNYko5C9s3pslv1ag=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR10MB7772
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-23_11,2024-05-23_01,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0
 suspectscore=0 mlxscore=0 adultscore=0 mlxlogscore=999 spamscore=0
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2405010000 definitions=main-2405230129
X-Proofpoint-GUID: OLuQ1K59yuKDogx7U8My6GMhX6_s740m
X-Proofpoint-ORIG-GUID: OLuQ1K59yuKDogx7U8My6GMhX6_s740m

Hi,

> -----Original Message-----
> From: Christoph Hellwig <hch@lst.de>
> Sent: Thursday, May 23, 2024 1:42 PM
> To: Gulam Mohamed <gulam.mohamed@oracle.com>
> Cc: linux-block@vger.kernel.org; linux-kernel@vger.kernel.org;
> axboe@kernel.dk; shinichiro.kawasaki@wdc.com; chaitanyak@nvidia.com;
> hch@lst.de
> Subject: Re: [PATCH V2 for-6.10/block 1/2] loop: Fix a race between loop
> detach and loop open
>=20
> On Tue, May 21, 2024 at 10:42:48PM +0000, Gulam Mohamed wrote:
> > Description
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>=20
> That's a weird way to format a patch.  Between this and the odd subject n=
ot
> matching patch 2 I was tricked into thinking this was just a cover letter=
 and
> patch 1 was missing for a while.  Please take a look at other patches/com=
mit
> and try to word it similarly.
I will take care of this in the next version.
>=20
> > V1->V2:
> > 	Added a test case, 010, in blktests in tests/loop/
>=20
> These kind of patch revision changelogs belong after the --- so that they=
 don't
> go into git history.  Or even better into the cover letter, which is miss=
ing here.
>=20
Sure. I will take care of this in the next version.
> > Signed-off-by: Gulam Mohamed <gulam.mohamed@oracle.com>
> > ---
> >  drivers/block/loop.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/block/loop.c b/drivers/block/loop.c index
> > 28a95fd366fe..9a235d8c062d 100644
> > --- a/drivers/block/loop.c
> > +++ b/drivers/block/loop.c
> > @@ -1717,6 +1717,24 @@ static int lo_compat_ioctl(struct block_device
> > *bdev, blk_mode_t mode,  }  #endif
> >
> > +static int lo_open(struct gendisk *disk, blk_mode_t mode) {
> > +        struct loop_device *lo =3D disk->private_data;
> > +        int err;
> > +
> > +        if (!lo)
> > +                return -ENXIO;
>=20
> ->private_data is never cleared, so the NULL check here doesn't
> make sense.
>=20
> > +        err =3D mutex_lock_killable(&lo->lo_mutex);
> > +        if (err)
> > +                return err;
> > +
> > +        if (lo->lo_state =3D=3D Lo_rundown)
> > +                err =3D -ENXIO;
> > +        mutex_unlock(&lo->lo_mutex);
>=20
> What if we race with setting Lo_rundown and that gets set right after we
> unlock here?
Similar race was mentioned by Kuai in his comments. We think these race con=
ditions can be resolved by bringing back the "lo->refcnt" ,
by reverting the commit a0e286b6a5b61d4da01bdf865071c4da417046d6 plus the a=
bove Lo_rundown check in lo_open.