Re: [bug report] ocxl: Add AFU interrupt support
On Tue, Feb 13, 2018 at 08:29:26PM +0100, Frederic Barrat wrote: > Hi, > > Thanks for the report. I'll fix the first issue. The 2nd is already on its > way to upstream: > https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dedab7f0d3137441a97fe7cf9b9ca5 > > (though we still have a useless cast in there; will fix as well). > > May I ask what static checker you're using? > These are Smatch warnings. regards, dan carpenter
Re: [bug report] ocxl: Add AFU interrupt support
Hi,
Thanks for the report. I'll fix the first issue. The 2nd is already on
its way to upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?id=dedab7f0d3137441a97fe7cf9b9ca5
(though we still have a useless cast in there; will fix as well).
May I ask what static checker you're using?
Thanks,
Fred
Le 13/02/2018 à 09:12, Dan Carpenter a écrit :
Hello Frederic Barrat,
The patch aeddad1760ae: "ocxl: Add AFU interrupt support" from Jan
23, 2018, leads to the following static checker warning:
drivers/misc/ocxl/file.c:163 afu_ioctl()
warn: maybe return -EFAULT instead of the bytes remaining?
drivers/misc/ocxl/file.c
111 static long afu_ioctl(struct file *file, unsigned int cmd,
112 unsigned long args)
113 {
114 struct ocxl_context *ctx = file->private_data;
115 struct ocxl_ioctl_irq_fd irq_fd;
116 u64 irq_offset;
117 long rc;
118
119 pr_debug("%s for context %d, command %s\n", __func__,
ctx->pasid,
120 CMD_STR(cmd));
121
122 if (ctx->status == CLOSED)
123 return -EIO;
124
125 switch (cmd) {
126 case OCXL_IOCTL_ATTACH:
127 rc = afu_ioctl_attach(ctx,
128 (struct ocxl_ioctl_attach __user *)
args);
129 break;
130
131 case OCXL_IOCTL_IRQ_ALLOC:
132 rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
133 if (!rc) {
134 rc = copy_to_user((u64 __user *) args,
&irq_offset,
135 sizeof(irq_offset));
136 if (rc)
^^
copy_to_user() returns the number of bytes remaining but we want to
return -EFAULT on error.
137 ocxl_afu_irq_free(ctx, irq_offset);
138 }
139 break;
140
drivers/misc/ocxl/file.c:320 afu_read()
warn: unsigned 'used' is never less than zero.
drivers/misc/ocxl/file.c
279 ssize_t rc;
280 size_t used = 0;
^^
This should be ssize_t
281 DEFINE_WAIT(event_wait);
282
283 memset(&header, 0, sizeof(header));
284
285 /* Require offset to be 0 */
286 if (*off != 0)
287 return -EINVAL;
288
289 if (count < (sizeof(struct ocxl_kernel_event_header) +
290 AFU_EVENT_BODY_MAX_SIZE))
291 return -EINVAL;
292
293 for (;;) {
294 prepare_to_wait(&ctx->events_wq, &event_wait,
295 TASK_INTERRUPTIBLE);
296
297 if (afu_events_pending(ctx))
298 break;
299
300 if (ctx->status == CLOSED)
301 break;
302
303 if (file->f_flags & O_NONBLOCK) {
304 finish_wait(&ctx->events_wq, &event_wait);
305 return -EAGAIN;
306 }
307
308 if (signal_pending(current)) {
309 finish_wait(&ctx->events_wq, &event_wait);
310 return -ERESTARTSYS;
311 }
312
313 schedule();
314 }
315
316 finish_wait(&ctx->events_wq, &event_wait);
317
318 if (has_xsl_error(ctx)) {
319 used = append_xsl_error(ctx, &header, buf +
sizeof(header));
320 if (used < 0)
Impossible.
321 return used;
322 }
323
324 if (!afu_events_pending(ctx))
325 header.flags |= OCXL_KERNEL_EVENT_FLAG_LAST;
326
327 if (copy_to_user(buf, &header, sizeof(header)))
328 return -EFAULT;
329
330 used += sizeof(header);
331
332 rc = (ssize_t) used;
^^
You could remove the cast.
333 return rc;
334 }
regards,
dan carpenter
[bug report] ocxl: Add AFU interrupt support
Hello Frederic Barrat,
The patch aeddad1760ae: "ocxl: Add AFU interrupt support" from Jan
23, 2018, leads to the following static checker warning:
drivers/misc/ocxl/file.c:163 afu_ioctl()
warn: maybe return -EFAULT instead of the bytes remaining?
drivers/misc/ocxl/file.c
111 static long afu_ioctl(struct file *file, unsigned int cmd,
112 unsigned long args)
113 {
114 struct ocxl_context *ctx = file->private_data;
115 struct ocxl_ioctl_irq_fd irq_fd;
116 u64 irq_offset;
117 long rc;
118
119 pr_debug("%s for context %d, command %s\n", __func__,
ctx->pasid,
120 CMD_STR(cmd));
121
122 if (ctx->status == CLOSED)
123 return -EIO;
124
125 switch (cmd) {
126 case OCXL_IOCTL_ATTACH:
127 rc = afu_ioctl_attach(ctx,
128 (struct ocxl_ioctl_attach __user *)
args);
129 break;
130
131 case OCXL_IOCTL_IRQ_ALLOC:
132 rc = ocxl_afu_irq_alloc(ctx, &irq_offset);
133 if (!rc) {
134 rc = copy_to_user((u64 __user *) args,
&irq_offset,
135 sizeof(irq_offset));
136 if (rc)
^^
copy_to_user() returns the number of bytes remaining but we want to
return -EFAULT on error.
137 ocxl_afu_irq_free(ctx, irq_offset);
138 }
139 break;
140
drivers/misc/ocxl/file.c:320 afu_read()
warn: unsigned 'used' is never less than zero.
drivers/misc/ocxl/file.c
279 ssize_t rc;
280 size_t used = 0;
^^
This should be ssize_t
281 DEFINE_WAIT(event_wait);
282
283 memset(&header, 0, sizeof(header));
284
285 /* Require offset to be 0 */
286 if (*off != 0)
287 return -EINVAL;
288
289 if (count < (sizeof(struct ocxl_kernel_event_header) +
290 AFU_EVENT_BODY_MAX_SIZE))
291 return -EINVAL;
292
293 for (;;) {
294 prepare_to_wait(&ctx->events_wq, &event_wait,
295 TASK_INTERRUPTIBLE);
296
297 if (afu_events_pending(ctx))
298 break;
299
300 if (ctx->status == CLOSED)
301 break;
302
303 if (file->f_flags & O_NONBLOCK) {
304 finish_wait(&ctx->events_wq, &event_wait);
305 return -EAGAIN;
306 }
307
308 if (signal_pending(current)) {
309 finish_wait(&ctx->events_wq, &event_wait);
310 return -ERESTARTSYS;
311 }
312
313 schedule();
314 }
315
316 finish_wait(&ctx->events_wq, &event_wait);
317
318 if (has_xsl_error(ctx)) {
319 used = append_xsl_error(ctx, &header, buf +
sizeof(header));
320 if (used < 0)
Impossible.
321 return used;
322 }
323
324 if (!afu_events_pending(ctx))
325 header.flags |= OCXL_KERNEL_EVENT_FLAG_LAST;
326
327 if (copy_to_user(buf, &header, sizeof(header)))
328 return -EFAULT;
329
330 used += sizeof(header);
331
332 rc = (ssize_t) used;
^^
You could remove the cast.
333 return rc;
334 }
regards,
dan carpenter
