Received-SPF: pass (google.com: domain of linux-kernel+bounces-215476-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <083cd51b-06d8-4618-8517-a2bc2fa31554@web.de>
Date: Fri, 14 Jun 2024 22:20:38 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Hao Ge <gehao@kylinos.cn>, linux-sound@vger.kernel.org,
 Jaroslav Kysela <perex@perex.cz>, Liam Girdwood <lgirdwood@gmail.com>,
 Mark Brown <broonie@kernel.org>, Shenghao Ding <shenghao-ding@ti.com>,
 Takashi Iwai <tiwai@suse.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
 Colin Ian King <colin.i.king@gmail.com>
References: <20240614024116.22130-1-hao.ge@linux.dev>
Subject: Re: [PATCH] ASoc: PCM6240: Fix a null pointer dereference in
 pcmdevice_i2c_probe
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240614024116.22130-1-hao.ge@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:2cv1dvyzcOM=;prvPWqHN9aM9/tNDwq0/vd1heIP
 2K9yrELvlf4JHKfNAKV4hED4qIGbVV7u7fZ7yiz8vQHme5l8fpcQ7FJNV8gG05hYyOnYgHY/d
 fJU1t34aQawC1VB22IuU6Zr+6JdWVBLmRJYBGRSTUNtw3K11lcDxFgYmQMPnqlG/LPVpPoIWx
 T86sI8SsAk+IwTqEbBBF60DiSoTx+GrygVkkP4M2WGxIohOqNFJPeWNbWlUEWQ2n0/btviuaI
 jqo4+cSwFUzagWweEF9MGNhw3t3wNwrWka0m8gsMplnFzW6Gv8rAWlVEATrEn3Xub4SN6Tmd3
 ijNuilMaeRqSYLXmhXDuSL9qpcU3rT9c09KBLvWuJV34Ss3bdZeP0ffZG1k+29p1AwezioD9z
 cNopnj4xhVPhutKw3pk88/YpbTI3tUcKGMMuuSmUNyNtM/rehSE24VpCBPzdRlbpkJrlvl9/5
 OV4eyDRGHElWi0FFVi1PdaW4o0fmd0MNZHJPEE6l+jbBslIWsNiAA34c3iZT2KCyJI4qEZqn0
 62LQRsEKMAS8cnMbrWzAOxynYyyaTTunZFwVaHxW8NR3VqPJmBVDqUDGf31EfNDtuk/PoidFk
 XqoJLRpXvbGvd0JshL/Gy/wANdjEum78YZMbFIlVQ4+cDh581tMgWZD6WoSw9hDbaH5lTTDMc
 mEzOggmTTMnhgxIyRTzv3Dg7roAkkqwSMX724FDWO86Y59ZxLtRLH/a0/G/3BmsDLJn6R6J1R
 0uzMCyPwY5b0myLO2wU4BViCwuV/oHzbvg4drdFX94+yYyTrYuCbI1bCqfcIe+hAD7OGkboqm
 tdtVOHU0cshReS2TJrjEZeE7T58w2Z5X5hJ4is3OOJJ/8=

> When devm_kzalloc return NULL,we return -ENOMEM directly to

This information fits to a coding style advice.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Do=
cumentation/process/coding-style.rst?h=3Dv6.10-rc3#n532


> avoid a null pointer that call pcmdevice_remove which will
> perform some operations on the members of the pcm_dev;

You pointed in an improvable way out that the label =E2=80=9Cout=E2=80=9D =
was questionable
after the assignment of a well-known error code at the beginning.
https://elixir.bootlin.com/linux/v6.10-rc3/source/sound/soc/codecs/pcm6240=
.c#L2077


Can a wording approach (like the following) be a better change description=
?

   The value =E2=80=9C-ENOMEM=E2=80=9D was assigned to the local variable =
=E2=80=9Cret=E2=80=9D
   in one if branch after a devm_kzalloc() call failed at the beginning.
   This error code will trigger then a pcmdevice_remove() call
   with a passed null pointer so that an undesirable dereference
   will be performed.
   Thus return the appropriate error code directly.


Can a summary phrase like =E2=80=9CReturn directly after a failed devm_kza=
lloc()
in pcmdevice_i2c_probe()=E2=80=9D be also helpful?

Regards,
Markus