All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@linaro.org>
To: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
Cc: intel-xe@lists.freedesktop.org
Subject: [bug report] drm/xe/uapi: Add UAPI for querying VMA count and memory attributes
Date: Thu, 28 Aug 2025 12:01:05 +0300	[thread overview]
Message-ID: <aLAa0ZEWi5mGed2S@stanley.mountain> (raw)

Hello Himal Prasad Ghimiray,

Commit 418807860e94 ("drm/xe/uapi: Add UAPI for querying VMA count
and memory attributes") from Aug 21, 2025 (linux-next), leads to the
following Smatch static checker warning:

	drivers/gpu/drm/xe/xe_vm.c:2298 xe_vm_query_vmas_attrs_ioctl()
	warn: maybe return -EFAULT instead of the bytes remaining?

drivers/gpu/drm/xe/xe_vm.c
    2240 int xe_vm_query_vmas_attrs_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
    2241 {
    2242         struct xe_device *xe = to_xe_device(dev);
    2243         struct xe_file *xef = to_xe_file(file);
    2244         struct drm_xe_mem_range_attr *mem_attrs;
    2245         struct drm_xe_vm_query_mem_range_attr *args = data;
    2246         u64 __user *attrs_user = u64_to_user_ptr(args->vector_of_mem_attr);
    2247         struct xe_vm *vm;
    2248         int err = 0;
    2249 
    2250         if (XE_IOCTL_DBG(xe,
    2251                          ((args->num_mem_ranges == 0 &&
    2252                           (attrs_user || args->sizeof_mem_range_attr != 0)) ||
    2253                          (args->num_mem_ranges > 0 &&
    2254                           (!attrs_user ||
    2255                            args->sizeof_mem_range_attr !=
    2256                            sizeof(struct drm_xe_mem_range_attr))))))
    2257                 return -EINVAL;
    2258 
    2259         vm = xe_vm_lookup(xef, args->vm_id);
    2260         if (XE_IOCTL_DBG(xe, !vm))
    2261                 return -EINVAL;
    2262 
    2263         err = down_read_interruptible(&vm->lock);
    2264         if (err)
    2265                 goto put_vm;
    2266 
    2267         attrs_user = u64_to_user_ptr(args->vector_of_mem_attr);
    2268 
    2269         if (args->num_mem_ranges == 0 && !attrs_user) {
    2270                 args->num_mem_ranges = xe_vm_query_vmas(vm, args->start, args->start + args->range);
    2271                 args->sizeof_mem_range_attr = sizeof(struct drm_xe_mem_range_attr);
    2272                 goto unlock_vm;
    2273         }
    2274 
    2275         mem_attrs = kvmalloc_array(args->num_mem_ranges, args->sizeof_mem_range_attr,
    2276                                    GFP_KERNEL | __GFP_ACCOUNT |
    2277                                    __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
    2278         if (!mem_attrs) {
    2279                 err = args->num_mem_ranges > 1 ? -ENOBUFS : -ENOMEM;
                               ^^^^^^^^^^^^^^^^^^^^^^^^

This is a weird check.  If args->num_mem_ranges is zero, then kmalloc()
will succeed with the ZERO_SIZE_PTR.  If it's 1, then
args->sizeof_mem_range_attr is quite small.  64 bytes.  The allocation will
succeed as well.  In real life err will never be set to -ENOBUFS.

    2280                 goto unlock_vm;
    2281         }
    2282 
    2283         memset(mem_attrs, 0, args->num_mem_ranges * args->sizeof_mem_range_attr);
    2284         err = get_mem_attrs(vm, &args->num_mem_ranges, args->start,
    2285                             args->start + args->range, mem_attrs);
    2286         if (err)
    2287                 goto free_mem_attrs;
    2288 
    2289         err = copy_to_user(attrs_user, mem_attrs,
    2290                            args->sizeof_mem_range_attr * args->num_mem_ranges);

copy_to_user() returns the number of bytes it failed to copy.  It should
be:

	if (copy_to_user(...))
		err = -EFAULT;

    2291 
    2292 free_mem_attrs:
    2293         kvfree(mem_attrs);
    2294 unlock_vm:
    2295         up_read(&vm->lock);
    2296 put_vm:
    2297         xe_vm_put(vm);
--> 2298         return err;
    2299 }

regards,
dan carpenter

             reply	other threads:[~2025-08-28  9:01 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-28  9:01 Dan Carpenter [this message]
2025-08-28  9:40 ` [bug report] drm/xe/uapi: Add UAPI for querying VMA count and memory attributes Ghimiray, Himal Prasad
2025-08-29  9:05   ` Dan Carpenter
2025-09-01  5:29     ` Ghimiray, Himal Prasad

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=aLAa0ZEWi5mGed2S@stanley.mountain \
    --to=dan.carpenter@linaro.org \
    --cc=himal.prasad.ghimiray@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    /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.