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 9800BC0218D for ; Wed, 29 Jan 2025 04:38:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2212728001B; Tue, 28 Jan 2025 23:38:06 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1D1FF28001A; Tue, 28 Jan 2025 23:38:06 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0725928001B; Tue, 28 Jan 2025 23:38:06 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id DCAC428001A for ; Tue, 28 Jan 2025 23:38:05 -0500 (EST) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 59675A0890 for ; Wed, 29 Jan 2025 04:38:05 +0000 (UTC) X-FDA: 83059232130.06.45BDCE2 Received: from mail-ot1-f52.google.com (mail-ot1-f52.google.com [209.85.210.52]) by imf14.hostedemail.com (Postfix) with ESMTP id 8B176100005 for ; Wed, 29 Jan 2025 04:38:03 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=EkolsvMe; spf=pass (imf14.hostedemail.com: domain of hughd@google.com designates 209.85.210.52 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1738125483; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Q0XQ2l+fMuLUvzO0JtwdKloIlRPEU2eLbB/nX4X3Wj8=; b=zPSQvb/f8cnqJpOIKpYmWQg9fBQS9cKsR470vUhar58VIYtRY5PCkA5HGAZjdVyD7jjF3O S90YBqPpSITivBR+R09s8IK6n6Uutu5ttpLZsx8v2ADwhcXcIQ+wo79oraEE0myX91BsBw I8DbD2mgxDbhjM6K61QFV/ZgCLL+VAI= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=EkolsvMe; spf=pass (imf14.hostedemail.com: domain of hughd@google.com designates 209.85.210.52 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1738125483; a=rsa-sha256; cv=none; b=1HqyTuqkkN35hynOcWo6Qik2UnEEyHlFq3vayV1eg96ejw7DuF/CgFRGTbLm5Cdai9DIC7 32KKil7UblJ5N2ck1Mvhc9Wd17cPB3+bgx31pl2MxTfSQA02p/LFPH12ojQ1wst82s9gIW qB5m71BMVzdhJsiXvlqzhgPOXwxpqYA= Received: by mail-ot1-f52.google.com with SMTP id 46e09a7af769-71e2bc5b90fso3419732a34.0 for ; Tue, 28 Jan 2025 20:38:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1738125482; x=1738730282; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=Q0XQ2l+fMuLUvzO0JtwdKloIlRPEU2eLbB/nX4X3Wj8=; b=EkolsvMejsHQF7yiwsfbOrIgbp174s59lfHqZ9p/ZsIk8kqng28J/IH0oMUcF1OsZW TtM9z1QrdSy/ro4/L0ANoHffc2dwmWZXQgB3L8/Uo9/JT/eiBUs/SqXoM34xwhnl3yG0 BfmDG/aCA92GgaGhIXTkRhsbVCnDCsh2pwgESC78sw+KUqDu8yZYG38OooCTdOYjirHm F1DAEZyg8n0899+1dfnCQwvMYAO6uL6bZvvRUkZ35oD9wNcZJSE9e/hT/DQhC05MagR6 nCWgCeTiTQ8Ko0yXCICF2wnGyqfdtxG10oZnurkQqHaSQZdDGNUJIDFTjHeA1/56T5lK lYFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1738125482; x=1738730282; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Q0XQ2l+fMuLUvzO0JtwdKloIlRPEU2eLbB/nX4X3Wj8=; b=WMG06PIM2bjhG8bcgoX9uWSStXZr35VTqXD51vORoQYMcmxgm2ble1GeD2vjpLQrPF 8HXkcN5SBSDlQ5fQgHIAhjixHE3eJnLpmQ2LbKjt0JYJvGeVhNK3kRGvhMp299vRhMWh 6jLQPUCwrIw1dpldu6oVZtaOlpgUcqeJV1fH4icUUOIAooozOGf4EbB76f5mHtddSUEJ mqsUaSuOVilcywwC5LBY4FjdYU7CB24jEkpwvwy4l1kBSQDE/349XUaPZHPFk7RL4z1Q qJ1p1F/gWCCdhI893oWDEwLrQVcJn9q+pTHsfaiE5OwDtNZ8CCUoVCOG1bLkSC95y/DQ IajA== X-Forwarded-Encrypted: i=1; AJvYcCVeXpmeacl3kCcgnjYhlIXqql7ct+k543JHqjDfHj4Bojxu4wjCy+rhrldTPR9jpvTc7RoOLFVjzg==@kvack.org X-Gm-Message-State: AOJu0YzizEBZMwaqm+5pPlsMy8aQtYaI8Qqhte+SCqXLejwGLsLEEYq8 k0bs2/eyQ6dCWw2xvvUXwZ9wRhJxtBgysZRmm3moso3ub/QUn3g4R9AT7GlxWQ== X-Gm-Gg: ASbGnctJP52T/GXA0tUC4wKI/B5nC2D5ivwaFZiz3Q3yRs5YfqqnnUxIHp/DrGKl/ow nKx9reJcMS+VNSLS3Dp3iOdeHzgRFJrKp3Jox6HwWuqI2zzaXFKqStax0VL/qckpfqjJbDCVlRs BmtcUgqFUsoaHfJUEEWXZPXnrXn+LL9/oNvqZNKFocjSC+Qtcz+aPCuCJO3evXfSVO1YTjBDhNe 1jT5h/aWarHKnn1lwoP2MtEdR91pIROfTR2gLfa2yE9AloO2mfxkIwYgK/yzepxHJhTSHuvurM/ fBTcJ697ORBltRo8EfpncxB65jjK42YT5LPpNU4e/BuyRLZ9cGmhM/lmzuknIPyS1tSB5/G/t5E = X-Google-Smtp-Source: AGHT+IFW4e6qYMjiUTKFtdFM7MOYs3D8TvcT4IhYcBc84MK2UL27j8GhF5H4LMJco+IbeZvImxjhkw== X-Received: by 2002:a05:6830:6886:b0:718:6cc:b5a2 with SMTP id 46e09a7af769-726568dca06mr951396a34.20.1738125482425; Tue, 28 Jan 2025 20:38:02 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id 006d021491bc7-5fa8b5548c7sm3441835eaf.17.2025.01.28.20.38.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Jan 2025 20:38:01 -0800 (PST) Date: Tue, 28 Jan 2025 20:37:51 -0800 (PST) From: Hugh Dickins To: Qasim Ijaz cc: hughd@google.com, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/shmem: Fix invalid PTR_ERR(NULL) call in shmem_xattr_handler_set() In-Reply-To: <20250128235408.11229-1-qasdev00@gmail.com> Message-ID: <16055fb0-bed3-e000-8c36-6e4886b08c9d@google.com> References: <20250128235408.11229-1-qasdev00@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 8B176100005 X-Stat-Signature: mcdmiup9q6gwmtjwxzpe95hir7wktift X-HE-Tag: 1738125483-102501 X-HE-Meta: U2FsdGVkX1/0kY0dXqmYtQF3B7+0fLPpCG6leC3xnnmb+fz67YPgnW1C/9zuteUWoZVL5c6NVb51OiWP7kIgj3Hmswx7mjSxZFZRG5SPrqbtR6Yczs1FxNhsaduUkZ6TUZKhJK89mjIaoQC/afkntY/Yqg4LWbJhFqnBy2noW5IYq/Ddzz1vuoVKO43mI0rBkT1qr/4zzvkRgCKb+MBskp5eFZW/q/NTyGAnz9+vXGdHPqYKbD0NF3bNyoqovoCQJ9umxJEb+HfrS2lO74wOyZ+iZqOADAgkDwJalV7U2Zo2H0yMNL2xaf+eAOksSn0Xb2DprKT7+1yRVDTuV5adGhGCWb4S4YT+SfbrLL4bpq57TPRJUekCdbwWxFJSundnApWQ+fx+8Z9p/oGzezLjsjqfj2uySkeooHC40F7is7Mawa1rQvzC7fNawP2YVV4HJMKV3QBZ8xdyywApwFq8wft+nrvfADN+6KFDPGgRFQXSDv5n8T3kbPRuS4D23dUD2JqUlEj0L1sYrufZV9JLp1fe31Cx+/JpJcsKm28u/06RJjMITv2k3cLRYl7vLT/GXIt6eASfMN73pXoHxY9PsXIxn6veeIvJmXqBUm6Kazx/Qym/uKRrN1odLC1deXMKSOOXalXiiZNN5UnU+8PQZmbVthyfihgE3KYuJeccfGYVsErIp3hLya98R4lH0rk4H6+ih7s5z47GeA1YCutuPHw8RnN3RJLUdFrS8dmmj9qVcMzSrqs25FPfgtZKx3DtUc7J7sjwiQ2T8HYj6AJ4XBIwTh7sSn7s4rX4wXX04YFG1kxvcdcLn/u/3BgM2Pus3iHyQA5d5BKdMaibNDzB/XxbN8FehXzeeisONcN5hL05dDVqwqPug0BW9QG3titpxrR2z176dCV5VQ00b+noARsM6k5vmbyMvU8lTLrtQtw3icF2ecc/8xmHdkV6D+QK1tsE/JN1e5ZI48AfMOt oPdIiZGU /zwNqqPLFjx8yNOy/RnTCpYmhB34OVRgj1CGTMQpI0qrWzCDiR4kTyFyQU8sRE+Bl6MEIEAgiUUDTceyQvaPRYQwR0HJZ8jRbmD9DGVbfQKBQhAIzAZ9Cr/XHt7syeTq/v8ePl/ihj/iizlE2Juj/RJq119wfBkzedJovA6HIRvQ+9wRdEdVPD8MLj9GLhZInw81IiAtSI2nBRZapA3G4OyQtCpTgXs07fGQ58dUDKozJVzvmeY8L46hfplZzzqmjlHt0UZkYufdn+cuCWga8UOFhyD+nG7IUExxZhBV238OJLz+kW9G/5E7er12U5Tmmy+7HIB1/aAPlxxmMPUWtb6QFTKjsyGCSS2j6J9b7oEE3xu4AtqRh49dCLZrsc4A2XUMWvjR2/gq7+i46UbFTmjXjzOpaf4qO31rlb/GGTIWZwH0= 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 Tue, 28 Jan 2025, Qasim Ijaz wrote: > In shmem_xattr_handler_set() if simple_xattr_set() succeeds and the pointer > returned is not an error pointer, then old_xattr will be set to NULL in > the body of the following if statement: > > if (!IS_ERR(old_xattr)) > > Later on shmem_xattr_handler_set() calls: > > return PTR_ERR(old_xattr); > > The PTR_ERR macro is used to extract an error code from an error pointer > and NULL is not an error pointer, PTR_ERR(NULL) simply results in 0. NULL pointer returns 0 for success: yes, that's what's wanted there. > > To improve correctness and readability, Correctness? Please explain - I don't see any incorrectness. Readability? Perhaps - I should not be the judge of that. > refactor the error handling > to have an explicit default return value of 0 (success) in "ret". > If simple_xattr_set() returns an error pointer, store its > error code in "ret". > > Signed-off-by: Qasim Ijaz > --- > mm/shmem.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) I prefer how it was written before. > > diff --git a/mm/shmem.c b/mm/shmem.c > index 532afd8e049c..3e97c7890aac 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -4143,6 +4143,7 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, > struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb); > struct simple_xattr *old_xattr; > size_t ispace = 0; > + int ret = 0; > > name = xattr_full_name(handler, name); > if (value && sbinfo->max_inodes) { > @@ -4158,7 +4156,9 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, > } > > old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags); > - if (!IS_ERR(old_xattr)) { > + if (IS_ERR(old_xattr)) { > + ret = PTR_ERR(old_xattr); > + } else { > ispace = 0; > if (old_xattr && sbinfo->max_inodes) > ispace = simple_xattr_space(old_xattr->name, > @@ -4168,12 +4171,13 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, > inode_set_ctime_current(inode); > inode_inc_iversion(inode); > } > + > if (ispace) { > raw_spin_lock(&sbinfo->stat_lock); > sbinfo->free_ispace += ispace; > raw_spin_unlock(&sbinfo->stat_lock); > } > - return PTR_ERR(old_xattr); > + return ret; > } > > static const struct xattr_handler shmem_security_xattr_handler = { > -- > 2.39.5