From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from devianza.investici.org (devianza.investici.org [198.167.222.108]) (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 E57D338887B for ; Thu, 23 Jul 2026 21:17:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.167.222.108 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784841438; cv=none; b=tWK3ai/XhXkXKdBDmXmUhzod4T0MDbruZRNWxoZ9jkOnHrjDWMf+xF3zCy4WyMFeQVMkb/B1PmxNYDJk/GEuWOZFAB91g3X5k8QHJrj/KTwdstWHOxuTvUX6rikJ6JaI+izNH3bsSMf0nFW8kjMPAq+AailDkmYzL86lUzdMtfU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784841438; c=relaxed/simple; bh=CjrDJwjZZOzdUkzr39g6R2VpCtnbKpQ48P/IO7fGmV8=; h=Date:From:To:CC:Subject:In-Reply-To:References:Message-ID: MIME-Version:Content-Type; b=I6ImrVZRDQ+15ENr6Dp9MXdyAMA2u3bC05257ENmmDxij/gI4w13lEs4YCuCxhkx84Yg0dO/uyxooeplMr8WQsvUfxO5FxbpIpLzU8Ukit41WWD0OygHfWAXBjvSLP0gUEgUp/xeXESiYITrRwJq/QxpaFCkHjYlE7E+Qdw3jDs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=grrlz.net; spf=pass smtp.mailfrom=grrlz.net; dkim=pass (1024-bit key) header.d=grrlz.net header.i=@grrlz.net header.b=dLepjjrv; arc=none smtp.client-ip=198.167.222.108 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=grrlz.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=grrlz.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=grrlz.net header.i=@grrlz.net header.b="dLepjjrv" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=grrlz.net; s=stigmate; t=1784841105; bh=E/H5kQ6cHsLb11Ui4Poaazj40STaiLH2kMOzbbiMAbw=; h=Date:From:To:CC:Subject:In-Reply-To:References:From; b=dLepjjrvnW+bxTHiJfcpukLbYDeYe3itOT7g73BsEAinCILTnhRcdwUf4aB6Fi2Sz tHRFb16DMjazrhCpCU8ATP6nqSiLlCueB28+CxHLfLIcflAFoQbqlvIrBnkNzMrKHe usP2MY05ZiviwBnN5B3LlCNzwUbBAQ5vbHCafGsk= Received: from mx2.investici.org (unknown [127.0.0.1]) by devianza.investici.org (Postfix) with ESMTP id 4h5kMP31YPz6vct; Thu, 23 Jul 2026 21:11:45 +0000 (UTC) Received: by mx2.investici.org (Postfix) id 4h5kMP08STz4vyJ; Thu, 23 Jul 2026 21:11:44 +0000 (UTC) Date: Thu, 23 Jul 2026 22:11:43 +0100 From: Bradley Morgan To: Andrew Morton CC: oleg.deomi@gmail.com, bsingharora@gmail.com, linux-kernel@vger.kernel.org Subject: =?US-ASCII?Q?Re=3A_=5BBUG=5D_taskstats=3A_REGISTER=5FCPUMASK_silently?= =?US-ASCII?Q?_truncates_the_last_byte_of_the_cpumask_string?= In-Reply-To: <20260723122825.4626f7e9efb30b22e2ac9d9b@linux-foundation.org> References: <5CBDF317-FB8B-4569-8B2F-96D350657054@grrlz.net> <20260723122825.4626f7e9efb30b22e2ac9d9b@linux-foundation.org> Message-ID: 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=utf-8 Content-Transfer-Encoding: 8bit On 23 July 2026 20:28:25 BST, Andrew Morton wrote: >On Thu, 23 Jul 2026 14:04:25 +0100 Bradley Morgan >wrote: > >> Hi Andrew, >> >> I dont think this actually fixes it... :( >> > >Thanks. Obviously I did't try very hard. > >> >> > - data = kmalloc(len, GFP_KERNEL); >> > + data = kmalloc(len + 1, GFP_KERNEL); >> > if (!data) >> > return -ENOMEM; >> > nla_strscpy(data, na, len); >> >> nla_strscpy() copies at most dstsize - 1 bytes, and dstsize is still >> len here. When the attr payload comes in without a trailing NUL, >> srclen == len >= dstsize and the last byte still gets cut off. The >> bigger buffer just adds a byte nothing ever writes to (the zero pad >> in nla_strscpy() only goes up to dstsize). >> >> Olegs report even spells this out in its suggested fix direction: >> allocate len + 1 *and* pass len + 1 as dstsize. The patch only >> picked up the first half. The fix is something like: >> >> data = kmalloc(len + 1, GFP_KERNEL); >> if (!data) >> return -ENOMEM; >> nla_strscpy(data, na, len + 1); >> >> or skip the dance entirely, nla_strdup() already does exactly this: >> >> data = nla_strdup(na, GFP_KERNEL); >> if (!data) >> return -ENOMEM; >> >> Btw the bug only bites when the sender doesnt NUL terminate the >> attr payload; senders that include the NUL (srclen gets decremented >> for the trailing NUL, so srclen < dstsize) were always fine. Thats >> probably why this survived 20 years. Might be worth a line in >> the changelog. And as the report already notes, the policy is >> NLA_STRING, not NLA_NUL_STRING, so a payload without the trailing >> NUL is legit input here. > >OK, I'll drop this. Please send along a fix sometime. > Hi Andrew, let me be honest, I CBA to do a new thread, here's my suggestion >From 0d5d963d0497e3d2c739e8b5c1403f707054e543 Mon Sep 17 00:00:00 2001 From: Bradley Morgan Date: Thu, 23 Jul 2026 21:09:22 +0000 Subject: [PATCH] taskstats: fix cpumask parsing cutting off the last character parse() hands nla_strscpy() len as dstsize, and nla_strscpy() copies at most dstsize - 1 bytes. When the attr payload comes in without a trailing NUL, srclen == len >= dstsize and the last character of the cpumask string gets cut off. Register "0-15" and you are silently listening on "0-1", exit data for the rest never shows up. The bug only bites when the sender doesnt NUL terminate the payload; senders that include the NUL were always fine (srclen gets decremented for the trailing NUL, so srclen < dstsize). Thats probably why this survived 20 years. And the policy is NLA_STRING, not NLA_NUL_STRING, so a payload without the trailing NUL is legit input here. Skip the kmalloc/nla_strscpy dance entirely and use nla_strdup(), which already allocates srclen + 1 and terminates. The nla_len() bounds checks stay as they were. Fixes: f9fd8914c1ac ("[PATCH] per-task delay accounting taskstats interface: control exit data through cpumasks") Reported-by: Oleg Deomi Closes: https://lore.kernel.org/CAByWkfZ6b1=3H9pwkz-dDQOs9cZaF-HYQ6b9Yb0=Hq2r1Vv_Pw@mail.gmail.com Cc: Balbir Singh Cc: Signed-off-by: Bradley Morgan --- kernel/taskstats.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/kernel/taskstats.c b/kernel/taskstats.c index d19bff9..632ca95 100644 --- a/kernel/taskstats.c +++ b/kernel/taskstats.c @@ -371,10 +371,9 @@ static int parse(struct nlattr *na, struct cpumask *mask) return -E2BIG; if (len < 1) return -EINVAL; - data = kmalloc(len, GFP_KERNEL); + data = nla_strdup(na, GFP_KERNEL); if (!data) return -ENOMEM; - nla_strscpy(data, na, len); ret = cpulist_parse(data, mask); kfree(data); return ret; -- 2.49.0 I better hope my mail client doesn't break the hell out of this! :) Thanks!