From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5B4FBC25B74 for ; Sun, 2 Jun 2024 16:35:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 99FDC6B00A8; Sun, 2 Jun 2024 12:35:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 927E56B00AA; Sun, 2 Jun 2024 12:35:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7C8BB6B00AB; Sun, 2 Jun 2024 12:35:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 5BCD26B00A8 for ; Sun, 2 Jun 2024 12:35:35 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id C8A9412048A for ; Sun, 2 Jun 2024 16:35:34 +0000 (UTC) X-FDA: 82186499388.23.5E3ADE8 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.48]) by imf23.hostedemail.com (Postfix) with ESMTP id EE074140019 for ; Sun, 2 Jun 2024 16:35:32 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CgoyD9o1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf23.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.48 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717346133; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=d3F/P98Vvj42F4uAlpuFuX2wB1pLK9ulXz/7ywvM+Wo=; b=43V3wRxa2/kG1DoFkpQ9jxAC02stfbIZ7YiO/3yI2670gCt7KzFtF/8VJ1rg4Z/VfM1Ka5 EN6Vf2nyWetB8uzf169gbNFe6au69KRwK/LDLzZaRNBWNTy4QJh7ohDDsS8bFHQdufyKYB ppKcVXpRuAFrT9HFBuIhG/f9X5NtHJk= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717346133; a=rsa-sha256; cv=none; b=lpZnvql2I0zPy//8bqBpuirIL/yeJQ8V3E8Qcc0niUPRCANatnJbl1+YKddsuoxe8IrESy HSbZfySdtOaz2vUQw0PRJ7LcmTWpZ9N+st7DrW9On2vkZClxHsOI0fIZ5Sj/JVkXbMsSbF ThqwAyc6DTCJIUCzzJVdN142/ffzKb0= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=CgoyD9o1; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf23.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.221.48 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-35dc1d8867eso2964606f8f.0 for ; Sun, 02 Jun 2024 09:35:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717346131; x=1717950931; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=d3F/P98Vvj42F4uAlpuFuX2wB1pLK9ulXz/7ywvM+Wo=; b=CgoyD9o1UCfYInjFp3mlBn0yrkbZZRPBacYf6jxUItL/Iq/Sas6TiJ/+8zHpKYAnki GGaesOZ2t1cPlci/BGbj2jEPadPkaFzGp6AMAxIsAsRndDLH+sb8vG8iU9FDQ+hIyaYz 6ArxFffh4TSR/b2zQprTAYgRimIp01oYOWsfE/HcGtmtkx0DGs3Y3Ig8sorqM4xOE1p5 A81J60OgSmlHGMBvJxjVffjkaRvxN3SRPZsNGBeKkXxHenymo0Ao4B1AYb4CpxqzzBAO +3eTwXJwTzPvvFyw++jwUX2BGhetbhrkTGdaQ/PoGUvY7CeIvx88/vyQmHGEftrE9PvK yfGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717346131; x=1717950931; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=d3F/P98Vvj42F4uAlpuFuX2wB1pLK9ulXz/7ywvM+Wo=; b=uSb0KldlcC9av+fZOa2kCvhrgN7TYGzlPxODAoldVdySxe4mKfe5ulCR187LJ+1h8w o9hajcBZK5RzG9hWYVc/7Sv41LUlSxQn+H1iXgKFYBKEjEWJZ/V0fCWbOF+n5C/QgtCF VxpeaJebSFv8UNVtqiBv+TOhdh/GagbLTegKIFOk0aqYgwQ8y1TKYqS6fAO8LbTpjL2W MecIjuxv8LxxmMca+qTK7fD88+E0L57Yu4mwOZ3cewPKxGV5HcLlLl4Z0rJZfAk7rtCX j2djSaEtQYJBTtNMqih988AtuxDUSoCFEf/0uGqON4M/vb+5OxN0A5pp1NbI9ky5jOsT X5Ng== X-Forwarded-Encrypted: i=1; AJvYcCX5eO2MWixhOKCWHTxPlWV6UxyIASU9S6/q5tkhjh06whwjte5rkaOUFYc0fTXWJ+QBuMOaI0i8tSrWsSdO9xkzw3Q= X-Gm-Message-State: AOJu0YzdEcP3uH57uO0Z7yO9SRlEm+ez2k8ZiBk3wlK4vLFHxiNJ89/g L9InmU3KyqD/GMKTyD+T7avizXaodfjf8lpip3ivDAtNAOmMRc3H/FEqMNOt9JfXTedfCsZ4OvR lxuH/6UTsrtgsdgdW1e0ibnHzEnE= X-Google-Smtp-Source: AGHT+IGhoPhpe51Y6Tp0rsrrRoTYtVnReDI2hgHxSRyGOSVnzBaep7p/mha6bjIlUVzy7MousDnj/U19epRohpFPBTE= X-Received: by 2002:a5d:4f01:0:b0:354:f66f:9292 with SMTP id ffacd0b85a97d-35e0f28441amr4419798f8f.28.1717346131116; Sun, 02 Jun 2024 09:35:31 -0700 (PDT) MIME-Version: 1.0 References: <20240602023754.25443-1-laoar.shao@gmail.com> <20240602023754.25443-2-laoar.shao@gmail.com> <87ikysdmsi.fsf@email.froward.int.ebiederm.org> In-Reply-To: From: Alexei Starovoitov Date: Sun, 2 Jun 2024 09:35:19 -0700 Message-ID: Subject: Re: [PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm() To: Yafang Shao Cc: "Eric W. Biederman" , Linus Torvalds , linux-mm , Linux-Fsdevel , linux-trace-kernel , audit@vger.kernel.org, LSM List , selinux@vger.kernel.org, bpf , Alexander Viro , Christian Brauner , Jan Kara , Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: n9mpsanu1zjwr3x1m8uqyf5ckk5umg6q X-Rspamd-Queue-Id: EE074140019 X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1717346132-94894 X-HE-Meta: U2FsdGVkX1/90IF3edEzxUrSNOf+uV650CHbHoJadWyiabcqlDnwdqvg7JviqABHX0QKPJL+mqeioZa4cFSmvTNiut0QEzncjhRPtTD7nzhvqr0AECgrMzEMqH9toV/SqMpM354W1na+/K0K+pFZeArZ9Lv+b7Nk6+r38rnAWx6WxFNnr5AnmV4nOFhd0WE1uD3MyMwbQ6f9sKjBq6dvZbITq24mIUKzZ+bcMvXxdmRXo2aMbC6y/bSIYI0wbnqJ8CZgnnvjj+RdjtckjlQhOleQjc3QzljyMLi4bhElcTf9z3ZBFP0uMXYBPzCAPeVMN6vnzRKA924lkE2PML3ltXmSmby2y7yIUjefxHduWVgR3hvJKDs82VG0zO8D1fvLCBEd4JyjKrCVDv9wXLmdnfDDBvw7YEkOCX/e37/qjfwTfllzdS9/5TNSrgUHhGsrJQeAoo0EHANMFHxYZJCA8JXaqvgsNo2/6x78ys0ObYsMCSyaW3UTjeP698BfPVL54Yr93TzHmwDll4xspnGbGWI1cVblOkemV3h3E8zfqaRpGY3ea3//8m5loK40cFhu6GARYkEftKfFMUqTZY/1mQzKenBefGoNmTtC9xFPmQfuqkyyt7iw+GJYDh8svqGctfcA+ISpRQcBk3SCbl8E/7FLqsmV8VzYyNlOQ4yKKGVOJsNXGub3c5RXNBsggSXJaaTNwedMe4QonyLuW3mww3IUgXOBgPY01uo8Mk6NVF3MUTDoq69Y1jG54yU0CqqOvbwnkBWAV0R6rJMpBtpZQIZEK9+PL9G53L6UY7hvSn/DuiFHIN1dshrB+0F8YG7m4QpeyPr2gx4oNeF0X3c+tzA/iG0JwjT3oqy03oAM3mBZKNX7nGtcYZGRBLoat0bj8DCVVb85a513Y9eNvs+ZHvRvSuPuWR4eG4gMNifumBfZu+/qgY1seBV2eDVYhiii+2cwbJcqcM5qvTlBcnW EqLDJUVm tJTZC7Rvzv3frVURz2Qni0TLwizWsrUkpz+nf/NYqFh30Xz9u68T9xJc20CRiGQNjmWgZLiD7tWFpIDxL4zkEyQFbPhnSFpOypR5Z10lazjSHwfD7/jQVjsF4DhVEeVolws9Vk+k0Nfvo5XhOkPbCHMmr+t6n3m268JofrGn8DrzNEz2k2Z0dfZPnLJ+ZhmgCqwsoUYlfDEHljA1pDCZbBuJ61FCN7usgKG07gsspgzyw9oNQJZROCcHsk78CLfRtHSmacWPg3ye34ObBCSxpDksN2ZRUz4q6WCyiEh4LxitphRgf7PB+NHvztKR2+LCHcDb1NqYPbVUVbZV000ag692mD5vpDBFlXuvWSgDN3jkDFVY7wNrVW0uQc5RLLxFz1b+djfSFrcPpd+1Lls+pLvsweDVjsnUTOEPHFLGzgZ+Mb4PhxhBNFz16d2rYmWpC1K/G8mXnHPQS2r5xnyYJIcFwSkG+ld4tZxFEFL2w6YANnaoRhLP9U0IG52sOrpGy7DkktoSXn8FfXioRQ84Rktc6kwDjJJaFegvT8WrKjI47nlnhkZ8HeVv1j/ZLwf4DoZ7yzKc/ch1nGdA= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Jun 1, 2024 at 11:57=E2=80=AFPM Yafang Shao = wrote: > > On Sun, Jun 2, 2024 at 11:52=E2=80=AFAM Eric W. Biederman wrote: > > > > Yafang Shao writes: > > > > > Quoted from Linus [0]: > > > > > > Since user space can randomly change their names anyway, using lock= ing > > > was always wrong for readers (for writers it probably does make sen= se > > > to have some lock - although practically speaking nobody cares ther= e > > > either, but at least for a writer some kind of race could have > > > long-term mixed results > > > > Ugh. > > Ick. > > > > This code is buggy. > > > > I won't argue that Linus is wrong, about removing the > > task_lock. > > > > Unfortunately strscpy_pad does not work properly with the > > task_lock removed, and buf_size larger that TASK_COMM_LEN. > > There is a race that will allow reading past the end > > of tsk->comm, if we read while tsk->common is being > > updated. > > It appears so. Thanks for pointing it out. Additionally, other code, > such as the BPF helper bpf_get_current_comm(), also uses strscpy_pad() > directly without the task_lock. It seems we should change that as > well. Hmm. What race do you see? If lock is removed from __get_task_comm() it probably can be removed from __set_task_comm() as well. And both are calling strscpy_pad to write and read comm. So I don't see how it would read past sizeof(comm), because 'buf' passed into __set_task_comm is NUL-terminated. So the concurrent read will find it. > > > > So __get_task_comm needs to look something like: > > > > char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *t= sk) > > { > > size_t len =3D buf_size; > > if (len > TASK_COMM_LEN) > > len =3D TASK_COMM_LEN; > > memcpy(buf, tsk->comm, len); > > buf[len -1] =3D '\0'; > > return buf; > > } > > Thanks for your suggestion. > > > > > What shows up in buf past the '\0' is not guaranteed in the above > > version but I would be surprised if anyone cares. > > I believe we pad it to prevent the leakage of kernel data. In this > case, since no kernel data will be leaked, the following change may be > unnecessary. It's not about leaking of kernel data, but more about not writing garbage past NUL. Because comm[] is a part of some record that is used as a key in a hash map.