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 13AC73BAD9B for ; Mon, 29 Jun 2026 10:16:43 +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=1782728205; cv=none; b=qWjJevFdxmSWmWj5VhNAS4de1tF0jTm0AOWO8bMo+CcZXm3xMKoqLQLLIblcJwbY8Ez6kBclgytVGGWSZwhbxTNunKQgWofe4AlHkNYJ6T9lGbOaHNKk7rEMiz7QpyxRlALnU17MNddqrF713WIu4XOkSLE+ncdmV1GvCgaUsQA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782728205; c=relaxed/simple; bh=nFq0UofNuPTqlv7acVN2XgwDssqTbNADEoZAFv5AKR4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Yo0fEW2ub/IktgZ8fVTyO0O56vM/FVH75sCtcz70X7ru7FFL95rk8uAk3fNGhG6hplKm/XnpSvm2qg6wR0a5GUqUpnvDHpOBjbqaZS6khz9HtsKuW5OVZU/TiY/G3+UJTAV4vWC2QPtBbrm0QLpDqYUTn0gqI54xXm83tcrpT4A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TvYmbSdO; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TvYmbSdO" Received: by smtp.kernel.org (Postfix) with ESMTPSA id D26E91F000E9; Mon, 29 Jun 2026 10:16:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782728203; bh=+CsIs1DJPAXe+YiF6JCSrE4IX+UynJ8nZdJo8ZYceTE=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=TvYmbSdOIHZtT31d6lRI38jqHxpSbW0J1P8qRmj3tIIyw26XC75DAQLuYZW9Bo60E YUVroNPiO/FYZz8BEv9GjcgpvHpNfoa0Rk9Y0MJhIvXHIl7wftN2mDOJK9WvxoFpEI XAKO6cAsbzu7m6QKeMKYl+R5Z+AuY3//kvB05KsLDbF89UcWFXS+SjFkcDJTnZyW2w 9Yl21BL9/fCPa14DV03hI0DqTVfTDqsP/9/hY7uLIQ8KNtClD1kiZWS8aI0HwkKv0t NVEF+LI6yUJ8vDw4LfU1Ic3lNXPBs17YpRq0BGAuYDxH+5V+HBOgM0kTJsQ8WFxQRL P8Zy8+x1Iao9Q== Date: Mon, 29 Jun 2026 11:16:36 +0100 From: Lorenzo Stoakes To: Yi Xie Cc: brauner@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ipc/sem: check nsops overflow in do_semtimedop() Message-ID: References: <20260629065705.99400-1-xieyi@kylinos.cn> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260629065705.99400-1-xieyi@kylinos.cn> On Mon, Jun 29, 2026 at 02:57:05PM +0800, Yi Xie wrote: > do_semtimedop() caps nsops against sc_semopm, but still multiplies it by > sizeof(struct sembuf) when copying from userspace. That product can > overflow, so copy_from_user() may not match what kvmalloc_objs() allocated. This is just incorrect, becasue for copy_from_user() to happen kvmalloc_objs() would be called -> __alloc_objs() -> size_mul() (saturates) -> -ENOMEM. > > Use check_mul_overflow() and return -E2BIG on overflow. > > Signed-off-by: Yi Xie > --- > ipc/sem.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/ipc/sem.c b/ipc/sem.c > index 5ec41de7e85b..791a0505923b 100644 > --- a/ipc/sem.c > +++ b/ipc/sem.c > @@ -86,6 +86,7 @@ > #include > #include > #include > +#include > #include > > #include > @@ -2225,6 +2226,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops, > struct sembuf fast_sops[SEMOPM_FAST]; > struct sembuf *sops = fast_sops; > struct ipc_namespace *ns; > + size_t sops_bytes; Maybe just len or size? > int ret; > > ns = current->nsproxy->ipc_ns; > @@ -2232,6 +2234,8 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops, > return -E2BIG; > if (nsops < 1) > return -EINVAL; > + if (check_mul_overflow(nsops, sizeof(*sops), &sops_bytes)) > + return -E2BIG; struct sembuf is 6 bytes is size, so you'd need some truly enormous ns->sc_semopm (defaults to SEMOPM == 500) and nsops to get an overflow here, and it'd only be the case on 32-bit systems also. But in any case as per above, kvmalloc_objs() -> __alloc_objs() -> size_mul() (saturates) -> -ENOMEM. So this check is redundant. > > if (nsops > SEMOPM_FAST) { > sops = kvmalloc_objs(*sops, nsops); > @@ -2239,7 +2243,7 @@ static long do_semtimedop(int semid, struct sembuf __user *tsops, > return -ENOMEM; > } > > - if (copy_from_user(sops, tsops, nsops * sizeof(*tsops))) { > + if (copy_from_user(sops, tsops, sops_bytes)) { > ret = -EFAULT; > goto out_free; > } > -- > 2.25.1 > Thanks, Lorenzo