Re: [PATCH v2] platform/x86/amd/hsmp: Remove NULL dereferencing code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


Hi Hans,

On 2/19/2024 6:18 PM, Hans de Goede wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

Hi Suma,

On 1/30/24 08:34, Suma Hegde wrote:
Do not log using dev_err() in case of !sock, which causes null pointer

Also remove unnecessary check "boot_cpu_data.x86_model >= 0x00", which is
always true because its an unsigned type.

Reported-by: kernel test robot <lkp@xxxxxxxxx>
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>

Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
Changes since v1:
Correct the email id for Naveen Krishna Chatradhi.

  drivers/platform/x86/amd/hsmp.c | 11 ++++-------
  1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp.c b/drivers/platform/x86/amd/hsmp.c
index 1baddf403920..1927be901108 100644
--- a/drivers/platform/x86/amd/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp.c
@@ -566,17 +566,15 @@ static ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
       struct hsmp_message msg = { 0 };
       int ret;

+     if (!sock)
+             return -EINVAL;
       /* Do not support lseek(), reads entire metric table */
       if (count < bin_attr->size) {
               dev_err(sock->dev, "Wrong buffer size\n");
               return -EINVAL;

-     if (!sock) {
-             dev_err(sock->dev, "Failed to read attribute private data\n");
-             return -EINVAL;
-     }
       msg.msg_id      = HSMP_GET_METRIC_TABLE;
       msg.sock_ind    = sock->sock_ind;

sock gets initialized like this:

         struct hsmp_socket *sock = bin_attr->private;

and bin_attr->private is setup before registering the file
and thus it will never be NULL.

So the correct fix would be to simply drop the check
rather then to move it.
Thank you for your review comment, I will send a patch to address this change.
@@ -739,8 +737,7 @@ static int hsmp_cache_proto_ver(u16 sock_ind)

  static inline bool is_f1a_m0h(void)
-     if (boot_cpu_data.x86 == 0x1A &&
-         (boot_cpu_data.x86_model >= 0x00 && boot_cpu_data.x86_model <= 0x0F))
+     if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
               return true;

       return false;
This bit looks fine but this really belongs in a separate patch
as it has nothing to do with "Remove NULL dereferencing code"

Ilpo has already merged it into relevant patch(platform/x86/amd/hsmp: Non-ACPI support for AMD F1A_M00~0Fh) and the change is available in review-ilpo branch.



Thanks and Regards,


[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux