All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <error27@gmail.com>
To: fkassabri@habana.ai
Cc: dri-devel@lists.freedesktop.org
Subject: [bug report] habanalabs: Timestamps buffers registration
Date: Fri, 6 Jan 2023 11:32:53 +0300	[thread overview]
Message-ID: <Y7fctbXR9oPd7K3J@kili> (raw)

Hello farah kassabri,

The patch 9158bf69e74f: "habanalabs: Timestamps buffers registration"
from Dec 23, 2021, leads to the following Smatch static checker
warning:

	drivers/accel/habanalabs/common/memory.c:2178 hl_ts_alloc_buf()
	warn: use 'gfp' here instead of GFP_XXX?

drivers/accel/habanalabs/common/memory.c
    2170 static int hl_ts_alloc_buf(struct hl_mmap_mem_buf *buf, gfp_t gfp, void *args)
                                                                       ^^^
"gfp" is never used.

    2171 {
    2172         struct hl_ts_buff *ts_buff = NULL;
    2173         u32 size, num_elements;
    2174         void *p;
    2175 
    2176         num_elements = *(u32 *)args;

This business of passing void pointers and pretending that
hl_cb_mmap_mem_alloc() and hl_ts_alloc_buf() are the same function is
a nightmare.

Create two ->alloc functions.  Split hl_mmap_mem_buf_alloc() into one
function that allocates idr stuff.  Create a function to free/remove the
idr stuff.  Create two new helper function that call the idr function
and then the appropriate alloc() function.

It will be much cleaner than using a void pointer.

    2177 
--> 2178         ts_buff = kzalloc(sizeof(*ts_buff), GFP_KERNEL);
                                                     ^^^^^^^^^^
Smatch is correct that it should be used here.

    2179         if (!ts_buff)
    2180                 return -ENOMEM;
    2181 
    2182         /* Allocate the user buffer */
    2183         size = num_elements * sizeof(u64);

Can this have an integer overflow on 32bit systems?

    2184         p = vmalloc_user(size);
    2185         if (!p)
    2186                 goto free_mem;
    2187 
    2188         ts_buff->user_buff_address = p;
    2189         buf->mappable_size = size;
    2190 
    2191         /* Allocate the internal kernel buffer */
    2192         size = num_elements * sizeof(struct hl_user_pending_interrupt);
    2193         p = vzalloc(size);
    2194         if (!p)
    2195                 goto free_user_buff;
    2196 
    2197         ts_buff->kernel_buff_address = p;
    2198         ts_buff->kernel_buff_size = size;
    2199 
    2200         buf->private = ts_buff;
    2201 
    2202         return 0;
    2203 
    2204 free_user_buff:
    2205         vfree(ts_buff->user_buff_address);
    2206 free_mem:
    2207         kfree(ts_buff);
    2208         return -ENOMEM;
    2209 }

regards,
dan carpenter

             reply	other threads:[~2023-01-06  8:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-06  8:32 Dan Carpenter [this message]
2023-01-09 15:07 ` [bug report] habanalabs: Timestamps buffers registration Farah Kassabri
2023-01-10  9:54   ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y7fctbXR9oPd7K3J@kili \
    --to=error27@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fkassabri@habana.ai \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.