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 7F1C9C25B74 for ; Sun, 2 Jun 2024 06:57:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B61836B0095; Sun, 2 Jun 2024 02:57:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B11086B0098; Sun, 2 Jun 2024 02:57:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9D7176B009A; Sun, 2 Jun 2024 02:57:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 7FEF06B0095 for ; Sun, 2 Jun 2024 02:57:03 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 938BFC026E for ; Sun, 2 Jun 2024 06:57:02 +0000 (UTC) X-FDA: 82185041484.24.D71BC85 Received: from mail-qk1-f177.google.com (mail-qk1-f177.google.com [209.85.222.177]) by imf22.hostedemail.com (Postfix) with ESMTP id D7DBFC0010 for ; Sun, 2 Jun 2024 06:57:00 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Lzp57nrC; spf=pass (imf22.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.222.177 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1717311420; a=rsa-sha256; cv=none; b=AP8e3YPljQqziEqjtjTgRK7wph0MHMuBXP2zJjOEYon0X+o0SaQvEhMlfMQkp3DcrnMR2W Mp1gYp+EVYNwQ5i4SKvk9fzx7/7ayx5w7XlqQhFhCPd7aWuKlfIK+msX2VGiqw/UZdq1py vqSyXwmk2g1bCkYo2Q9WnS/Zi8n5Ysc= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=Lzp57nrC; spf=pass (imf22.hostedemail.com: domain of laoar.shao@gmail.com designates 209.85.222.177 as permitted sender) smtp.mailfrom=laoar.shao@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1717311420; 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=AAUPbRzuyrXUUhAbCV+RswkSNoR9RTLDNyX2Ucwmfqo=; b=ZU5/k0tVWKAFmI11+NKwAw6hdnXcQvJqDsAgiRmWFVZHG1tzd7uFz5vZuri4gtsF0cx3nx z3jjeJyO3AxtYScZAsstnrmJQu3zpMaD8BvVXd96JFWwIEGfK4KstLx5WEl9FGnVOjCKoG XtLDS00Eo7AlK9j4ycpPlVB7bqmx/ck= Received: by mail-qk1-f177.google.com with SMTP id af79cd13be357-794ab4480beso256471685a.1 for ; Sat, 01 Jun 2024 23:57:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1717311420; x=1717916220; 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=AAUPbRzuyrXUUhAbCV+RswkSNoR9RTLDNyX2Ucwmfqo=; b=Lzp57nrCPL2U5toVMB1ishdLtP+q5oIfdIADPzt3QuUmxzerL01Q3UIeMkV1DiugKB TisFt2AX0hg6C3IqeyLTNkYrRP2fMaFBeYbqTNExAFT4YW6I9UdM6IuLBgRpj0P0mx0F ZpQvvHZaXki3b2HygJWx96nC15qNZeBh4Oiy3yVFA9Y7chPAaR1FUY+bECnuR+gsC3FA FgHhcb0Jd0fA8DLpRCthf+tVCOy6UpXAlkudhSCKk/uhMdKvwIq/Bqxonk7pWM3uO8s0 uHmRHMZJHQUOV2o1VPA1KZ90dQAQ+3iHTdQsLSiJWp168YDVypO6nvN8QgDJVHZ04cjQ WhuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717311420; x=1717916220; 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=AAUPbRzuyrXUUhAbCV+RswkSNoR9RTLDNyX2Ucwmfqo=; b=u1bfiC30JfNzi+7Qlp7iJZrJwCfHBvHf2oRKt3c7lNb/QrC2493y/vZxJT3fbKd2nJ Jzl8gmoD8bfcA+En3gM7txYesf4ITFjlTV7Wd459C8Pji/OH36sfwQijCOGphZKmn2nE MqLGgtHzEZ1zhdDkoURcfa/a0kyTAz0JAFCL2mUZ7tyXHKsanfzOuuX3NIfs7gSQ3ioL oOtgjL5Vd+NDmfOzD8/hAXiuDI1m5uvuD03svXsZ8Xr8LiKILvyzR2aRj9cjvpL2p/xr s5DoWhzGHRy7Q/o/C5YSWxEiVX9L8D2GbClBO12fHYcWL1GoTtHmZPtIkxffYW71+csQ lRhw== X-Forwarded-Encrypted: i=1; AJvYcCXDqpzKZdA+zxiGkR4Nu/370iqUXFY5BPN29eeeqjK5vlW9zZfcANvogkrtGmeccPGO6RuqN0H+D7HYsO6/LlbFopk= X-Gm-Message-State: AOJu0Yy/76Di852pNcRmbWnkOZDsoHOutyvpLAYwgndoRGYfiIcv98q6 O1ty2BHKQGv+H7evbh5ykj6LoEDzX0OeFAd2cF4K2TchnPZWgLudPFetZmkApReMqV+3LJK1Tjy 5XNkhOGCFlEaZ1WQKP7Nh4g7O/LY= X-Google-Smtp-Source: AGHT+IHBO8CcAWYey7pRZVURNsZzP8hqqeNI4aNYwlr3UTd/2FxO0+zZYuavJGm6rllryB0FhnqOhCZpQ6HL1oaZSiY= X-Received: by 2002:a05:6214:4891:b0:6ab:9492:7d89 with SMTP id 6a1803df08f44-6aecd6f054bmr77058646d6.52.1717311419907; Sat, 01 Jun 2024 23:56:59 -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: <87ikysdmsi.fsf@email.froward.int.ebiederm.org> From: Yafang Shao Date: Sun, 2 Jun 2024 14:56:23 +0800 Message-ID: Subject: Re: [PATCH 1/6] fs/exec: Drop task_lock() inside __get_task_comm() To: "Eric W. Biederman" Cc: torvalds@linux-foundation.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, audit@vger.kernel.org, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, bpf@vger.kernel.org, Alexander Viro , Christian Brauner , Jan Kara , Kees Cook Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: D7DBFC0010 X-Stat-Signature: ehcaxhuz9pag3r13odkw8p59nmpuq7bb X-HE-Tag: 1717311420-530388 X-HE-Meta: U2FsdGVkX19Aq2T3DVDdLjfxzZ2/MRoEkRtprbU+B8IiZq/NicZ9a8rdERd9IYw10W5h0v5nsewTc472izRXvTgQ6NC36KJ3/tYASaz6ytgxt9aYQX8YuW52ugACVuaP3OOew+u9II+qtNvbUBTnAZ4tqjNAhHTURQwkmxuLRArS3RhycETKTBrrK0YEmQMgLY2k1C0eb6p9cW9Bfy4WajyQ5cv++6Ot+qJkm4b7eT38b9x2cgiGVVKtH/axcGmlQz7XW5F+F0ZUntcrzsv2R7ktPuQuOWUIqkU+oic4NKvDxg13unruHt+44WHD8q8biYo69D4g2kT3E4gFTn13aF4AKUWcFHpMxxJjPK7eiCf4WeIyKBhvMmkl1ilKFje7SloT8dKakEXvByLYVqEANWrZU+9fvQ5FJ9Kl+pzj4cWVOHNuDlmizHVf455oSqZ14sIUEj3JFwD/ssA9hXWUhKohFvm2szMRelQl37k0uuCKXTokGhTGA7gI5ZFcTwQiZBRr0swCpZiaJIpkA03S+gObhnuHPpLPRSylEHPo1xjsH8r6+zHy2oWpzVnfWiaO3mu4roR2FYqsrtWH5MQVZ8uTvDvP0qtdyA8Rm3Jrt5szlqtPU+0lSbQ5UmLPixMNyvEEVRUzueArHLFS6f7wrtxhH4AZRJbw4idgmCvlVs/V4z3KelC7iJjyZqIUUOtRXfVentB5jsBS39cN/FqhNZENDuYLGE22/xxGf0l1Bc2LkfsM8b0/saTzREN07HdQDqqsYXCLWsOsEpAbH44hkQUEjWSTQrqZoGxWRRiHqtx2jiaHRB2LuzYz90msx9l+tOc9Ll5V+lvS8C3SJDV0jiZPETK9xiZzETC6V5EmBNlffJ3jvx3gEY89I8myDtg0ZxwBKTwxPvgFzK8qZS1UqHiJJPgQssESbQ1mZWBYF1ubmt4I9hunRNu0WaMumPibDmXwHT4ODi3Mck5+oE9 95xcqi31 a1Ki9GrwZmChIdf0mn/r7kBYYOqZmx06iUrji41pl/rLacOxZKjsfJ+Y08LzkHKyabSgdSWHlMin7IZGnOdz3k7HYlYCUK5CW24kta/uX2YmBFeR7F4b6LW95SDoz/C7LkxE+Ey4QnhX70ffjs6/KQnKvMhbPF/UIWWfj9s+A97n2kx1aBjTR1x9w+O+SRjqrV6wEEToLyDwTmbym6Om66Hzit7eZCkZxHKl0pX10FRpLT57ZiUvFCRG2x6RpoItBxYxJsve4+cGsjaC9zaSFEhUpZWeJBibyrOzuoXHB4y/j1YFEPm1L8kbtjX/abXuBDRcPGWR0Y91gPjVKg+oviOhAStJQDl/QAt3iFH4RfNbjkbJvRqVAc2Q6oEnxCW/PEP6CDqgEcMqafdrTiP16vD4IWSxP5u2JV4+5rSPbN+Cl41LF+81rTDWNHUM4f72bBPWYJ/d5olx2rRCiVdhW/kHtHVmTC5QUiSIiMoOf0UMN8c8NP6myLCvMIfoeyLjqL999haWdXYce+jg= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, 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 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 lockin= g > > was always wrong for readers (for writers it probably does make sense > > to have some lock - although practically speaking nobody cares there > > 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. > > So __get_task_comm needs to look something like: > > char *__get_task_comm(char *buf, size_t buf_size, struct task_struct *tsk= ) > { > 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. > > If people do care the code can do something like: > char *last =3D strchr(buf); > memset(last, '\0', buf_size - (last - buf)); > > To zero everything in the buffer past the first '\0' byte. > --=20 Regards Yafang