Received-SPF: pass (google.com: domain of linux-kernel+bounces-37203-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 24 Jan 2024 17:48:52 +0300
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Erick Archer <erick.archer@gmx.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Franziska Naepelt <franziska.naepelt@googlemail.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Johannes Berg <johannes.berg@intel.com>,
	Jeff Johnson <quic_jjohnson@quicinc.com>,
	Aloka Dixit <quic_alokad@quicinc.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-hardening@vger.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: Use kcalloc() instead of kzalloc()
Message-ID: <b40e12f6-176a-4bf6-8ab0-d1cf783033a0@moroto.mountain>
References: <20240119173900.11035-1-erick.archer@gmx.com>
 <a5d8a5f1-432d-46f0-84fe-7b5b22ff5f32@moroto.mountain>
 <20240122181654.GA2834@titan>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240122181654.GA2834@titan>

On Mon, Jan 22, 2024 at 07:16:54PM +0100, Erick Archer wrote:
> Hi Dan,
> 
> On Mon, Jan 22, 2024 at 09:55:11AM +0300, Dan Carpenter wrote:
> > On Fri, Jan 19, 2024 at 06:39:00PM +0100, Erick Archer wrote:
> > > As noted in the "Deprecated Interfaces, Language Features, Attributes,
> > > and Conventions" documentation [1], size calculations (especially
> > > multiplication) should not be performed in memory allocator (or similar)
> > > function arguments due to the risk of them overflowing. This could lead
> > > to values wrapping around and a smaller allocation being made than the
> > > caller was expecting. Using those allocations could lead to linear
> > > overflows of heap memory and other misbehaviors.
> > >
> > > So, use the purpose specific kcalloc() function instead of the argument
> > > count * size in the kzalloc() function.
> > >
> > > Also, it is preferred to use sizeof(*pointer) instead of sizeof(type)
> > > due to the type of the variable can change and one needs not change the
> > > former (unlike the latter).
> > >
> > > Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments [1]
> > > Link: https://github.com/KSPP/linux/issues/162
> > > Signed-off-by: Erick Archer <erick.archer@gmx.com>
> >
> > I quite often write responses to patches and then never send them.  I
> > wrote this response and debated sending it but in the end I decided to
> > send it because you have sent multiple patches.  If you had only sent
> > one patch then I wouldn't have bothered.
> 
> My intention is not to bother anyone. I'm a linux kernel developer newbie
> and I try to do my best.
> 

Yeah.  It's not a problem, we all started as newbies.  I guess what I
was trying to say is that if you're going to be sending a lot of patches
then it's worth explaining this but if you're only sending one then I
wouldn't bother.

I don't really expect people to figure out all the code, it's just that
if you can see the effect of a patch right away then, please, include
that in the patch.  Even when it's like in this case, "This patch should
have no effect on runtime."  That sort of information is still useful.

Or if you can't figure out the implications that's also useful
information to me as a reviewer.  Just put those I don't know comments
under the --- cut off.

regards,
dan carpenter