From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f48.google.com (mail-ot1-f48.google.com [209.85.210.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8695411713 for ; Wed, 29 Jan 2025 04:38:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738125485; cv=none; b=s2U6i65JvH/hEyb2X+z5IleVE5Fya+LiEvoYYa7ZW//88ST7r+CZwotDq3fS9WUgixMIAGmU1wM9Z6Ysxr0tPf9oOo0d/mvDfoXQRNbKrBYlX2dKb2p7bLp4Ph0nsioomA/3LfBjc50e7jgMMqJcRIDm1JVA69eLdjas1a3XGwc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1738125485; c=relaxed/simple; bh=oVpAtCtJSTmrNb64DFMEWfOfIX7cP/lhjlxF0Bywijc=; h=Date:From:To:cc:Subject:In-Reply-To:Message-ID:References: MIME-Version:Content-Type; b=SCFhACeMUYCklREqEdNxWihaUlZ0xnC6KXjLpPELEZkPb7GQ1xXfgAI6yiMvTW+KTu+0+7HBnQTOZO52MmWcfzEWgQeHdnArQh1JiXTjqYV1GMXVeHJ0QD/f0P6wlxI9psh9bTme2qj1X9k3zGWEvb993BccGQV92F1lRdj4tJw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=LWq7dr9K; arc=none smtp.client-ip=209.85.210.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="LWq7dr9K" Received: by mail-ot1-f48.google.com with SMTP id 46e09a7af769-71e15717a2dso3382711a34.3 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=vger.kernel.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=LWq7dr9KOVp78fIe5I3g2fBaRtUacdP+MD4BloKYUUz2eZcIcOe4uEB8cH3IrXOhoo OxpfcikHcKe81sZqkgrEWaCEz6JkSfWVqcF/yZ71GYSI3dQIgNKMhRBOVaQWmetkNKXX F2rT9OkeQSGKHvwKBa9elcAEBXIPAVaNyq4vg2EUMI27oQexYxsHhwYJkmm/AfdROdnU iEQQJZopp+cWLEMvQ8u1Se9L5yWit34H/flJASImjGymyJ3acGTGpcBh/XIzYK6rsPvg mvVvWjCR0iPG+vfZpi8uQ0wFA+MLABiEuB/yEhHohzKxMThaXS5JD39ksXG7DdbJCozw skVA== 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=q+9x1BlerOvBmdnw+upJi/DmWXBd1jyCdYo4CWvG6hKUbSFBs/caqIMHvTJzPJUj/C pFvuQ71KnhOXTW0ElSlXMigUsXvrewvVgdkg2U4BGIyzaEXlxQBWj5CsmzMGpf2TEMIu RhRrHIeWv0ByQ19HEdbJhbvT2foj9eMOMuO43hVjoi2vVc80z6pA98RFiHIlCApiiCHV 1IlFEnQJbZQzBBGG1OyeujsVEZDDgD63RXx/FEFmlxG39ZlfDCI0Nn9F5hrtSAokgqrb kLOjZJKqiBnigYMvNV2k7cBtzgA7I/v38xKdG+O7Uv8q2ZzKZH/oEkdEIGIpKN6VtcOc 3B4A== X-Forwarded-Encrypted: i=1; AJvYcCWhcMd0atDsQSujD0FR3cwP5MyMRyqqFdtQon3xr5H/D99al/nzN87v9DK8kZQBN/YzPGuT0AJYU5yiXMs=@vger.kernel.org X-Gm-Message-State: AOJu0YwWH+7KrAwVstH1ACHJsOVk2f8IeHjIog3mmee2ZuYfueOaVWox HibFD95i7rEYr3A5CeOqskXLYZxgH5Fjoy3AWqONA+U06EIfIs2B2fkliTfCLg== X-Gm-Gg: ASbGnctW+bpc3zT1piEpZvlQvhz5HtCLweFv3pi3EDubLJ80GLN9Ul9LxQJ5t6GwTJ2 +NX0YGQd1sps2FlD0zDvAT/0rCrUtS8i3dgrkke64d5ayT3qfO+O4OSIFcnnAPJ2qXK5DOoI0eP t0gvQ/bxNsXH1hta/RAGjp8pS6FldaJPVAeBii4Q4C+zDT8zLZWUwxiiQxjLhiT2NikZXhs7ueK 0hE3Q6UMB7dt9D7jSVT/0bTxOOW9dt5MzIEyFAf/X85f3gFyAE0ppTQ63DkRViD7jrClRiWGNmW /1eGIFF0PL2/0aALX13vR94GgJsiOVZmZmS0UtiYA/njowFEjt5/dFpuAouCJ4sdrimtdtOWUDs = 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> 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 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