From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by smtp.subspace.kernel.org (Postfix) with ESMTPS id E15D218C933
	for <linux-kernel@vger.kernel.org>; Sat, 18 Jul 2026 03:18:36 +0000 (UTC)
Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18
ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;
	t=1784344717; cv=none; b=B8g7owaiF3fLJ/Cy6qFtd+g8/KhpmxvhkgFmf9439Yg0sJNM4bQqx9q7MwTHN+PjyrB17rAMu71fXcXWKzVWakABfWx9JTWx8qiuklleZIoIxViQeHHaXbkXqEr97+p5VEK7nGx2dR6aBeRQXZltOrIhvyKi75tkJE19q7sQMz0=
ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org;
	s=arc-20240116; t=1784344717; c=relaxed/simple;
	bh=+p64rZ5ITpyjj7XHau1oyTBtlcHi1pbAWblBTc4W2aA=;
	h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References:
	 Mime-Version:Content-Type; b=cu3hiaGSgDiDWv7iyId/U2tJV72KyqMgHvNaYp+oiUtb8zWCo7aUXUGvs7OzzjorumZGFTPsQe8QWQPm3W2rknFmccTLaN8K+FCtD2m28LGJhlWByDdvpRoNtRLbSkWSmRfHXxbuc38aNYEqUS0HArfKDd1t69dh/dpxyyesNO4=
ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b=W147R/Rx; arc=none smtp.client-ip=100.103.45.18
Authentication-Results: smtp.subspace.kernel.org;
	dkim=pass (1024-bit key) header.d=linux-foundation.org header.i=@linux-foundation.org header.b="W147R/Rx"
Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56F5D1F000E9;
	Sat, 18 Jul 2026 03:18:36 +0000 (UTC)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
	d=linux-foundation.org; s=korg; t=1784344716;
	bh=UidD7weOC8Cmrxv/dMXOSbbsmiWnBxlMqklPFGaX6UI=;
	h=Date:From:To:Cc:Subject:In-Reply-To:References;
	b=W147R/RxDe73lqHsfoqYpUogcYPJbWZ77qKzCBDWXjzwX5Shay7KTCbpydM5bQBvn
	 LubbKLKvVuXTwz6hZQ/uS1Ou/Eq+zOVuNaNdXGPE/ANb89NXp9xHCgQy5m0VKl3SrP
	 bHjulrY8e6MEwDMlKhjHwUxOV3eO69wzk2bUH3mE=
Date: Fri, 17 Jul 2026 20:18:35 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Yi Xie <xieyi@kylinos.cn>
Cc: ljs@kernel.org, linux-kernel@vger.kernel.org, Davidlohr Bueso
 <dave@stgolabs.net>
Subject: Re: [PATCH v3] ipc/shm: check shm_lock() in do_shmat cleanup
Message-Id: <20260717201835.44be64d0e0ab3f9a0115eb01@linux-foundation.org>
In-Reply-To: <20260716075330.96378-1-xieyi@kylinos.cn>
References: <20260716075330.96378-1-xieyi@kylinos.cn>
X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu)
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
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Thu, 16 Jul 2026 15:53:30 +0800 Yi Xie <xieyi@kylinos.cn> wrote:

> do_shmat() calls shm_lock() in the out_nattch branch and
> immediately dereferences it, however shm_lock() can return an
> error.
> 
> Check for an error and handle it if there is one.

Thanks.

ipc/ doesn't have a listed maintainer, but Davidlohr usually helps out.

> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -1677,12 +1677,16 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
>  out_nattch:
>  	down_write(&shm_ids(ns).rwsem);
>  	shp = shm_lock(ns, shmid);
> -	shp->shm_nattch--;
> +	if (IS_ERR(shp)) {
> +		err = PTR_ERR(shp);
> +	} else {
> +		shp->shm_nattch--;
>  
> -	if (shm_may_destroy(shp))
> -		shm_destroy(ns, shp);
> -	else
> -		shm_unlock(shp);
> +		if (shm_may_destroy(shp))
> +			shm_destroy(ns, shp);
> +		else
> +			shm_unlock(shp);
> +	}
>  	up_write(&shm_ids(ns).rwsem);
>  	return err;

Sashiko says that shm_lock() cannot actually fail in this situation.

	https://sashiko.dev/#/patchset/20260716075330.96378-1-xieyi@kylinos.cn

It's pretty horrid (and fragile) for calling code to "know" this is the
case, so I think it's good from a maintainability point of view to add
code to handle this cannot-happen case.  Or, better, to do some
refactoring to eliminate this situation without adding overhead.

If we decide to keep the code as-is then it definitely needs a code
comment explaining why we don't need to check the shm_lock() return
here.

Sashiko also suggests that the proposed error-return handling is inaccurate.