* [bug report] ocxl: Add AFU interrupt support
@ 2018-02-13 8:12 Dan Carpenter
2018-02-13 19:29 ` Frederic Barrat
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2018-02-13 8:12 UTC (permalink / raw)
To: fbarrat; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ocxl: Add AFU interrupt support
2018-02-13 8:12 [bug report] ocxl: Add AFU interrupt support Dan Carpenter
@ 2018-02-13 19:29 ` Frederic Barrat
2018-02-14 7:29 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Frederic Barrat @ 2018-02-13 19:29 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linuxppc-dev
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ocxl: Add AFU interrupt support
2018-02-13 19:29 ` Frederic Barrat
@ 2018-02-14 7:29 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2018-02-14 7:29 UTC (permalink / raw)
To: Frederic Barrat; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-02-14 7:29 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-13 8:12 [bug report] ocxl: Add AFU interrupt support Dan Carpenter
2018-02-13 19:29 ` Frederic Barrat
2018-02-14 7:29 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).