Hello Matthew Bobrowski,
The patch f644bc449b37: "fanotify: fix copy_event_to_user() fid error
clean up" from Jun 11, 2021, leads to the following static checker
warning:
fs/notify/fanotify/fanotify_user.c:533 copy_event_to_user()
error: we previously assumed 'f' could be null (see line 462)
fs/notify/fanotify/fanotify_user.c
401 static ssize_t copy_event_to_user(struct fsnotify_group *group,
402 struct fanotify_event *event,
403 char __user *buf, size_t count)
404 {
405 struct fanotify_event_metadata metadata;
406 struct path *path = fanotify_event_path(event);
407 struct fanotify_info *info = fanotify_event_info(event);
408 unsigned int fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
409 struct file *f = NULL;
410 int ret, fd = FAN_NOFD;
411 int info_type = 0;
412
413 pr_debug("%s: group=%p event=%p\n", __func__, group, event);
414
415 metadata.event_len = FAN_EVENT_METADATA_LEN +
416 fanotify_event_info_len(fid_mode, event);
417 metadata.metadata_len = FAN_EVENT_METADATA_LEN;
418 metadata.vers = FANOTIFY_METADATA_VERSION;
419 metadata.reserved = 0;
420 metadata.mask = event->mask & FANOTIFY_OUTGOING_EVENTS;
421 metadata.pid = pid_vnr(event->pid);
422 /*
423 * For an unprivileged listener, event->pid can be used to identify the
424 * events generated by the listener process itself, without disclosing
425 * the pids of other processes.
426 */
427 if (FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
428 task_tgid(current) != event->pid)
429 metadata.pid = 0;
430
431 /*
432 * For now, fid mode is required for an unprivileged listener and
433 * fid mode does not report fd in events. Keep this check anyway
434 * for safety in case fid mode requirement is relaxed in the future
435 * to allow unprivileged listener to get events with no fd and no fid.
436 */
437 if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
438 path && path->mnt && path->dentry) {
439 fd = create_fd(group, path, &f);
440 if (fd < 0)
441 return fd;
442 }
"f" is NULL on the else path
443 metadata.fd = fd;
444
445 ret = -EFAULT;
446 /*
447 * Sanity check copy size in case get_one_event() and
448 * event_len sizes ever get out of sync.
449 */
450 if (WARN_ON_ONCE(metadata.event_len > count))
451 goto out_close_fd;
452
453 if (copy_to_user(buf, &metadata, FAN_EVENT_METADATA_LEN))
454 goto out_close_fd;
^^^^^^^^^^^^^^^^^
This is problematic
455
456 buf += FAN_EVENT_METADATA_LEN;
457 count -= FAN_EVENT_METADATA_LEN;
458
459 if (fanotify_is_perm_event(event->mask))
460 FANOTIFY_PERM(event)->fd = fd;
461
462 if (f)
^^^
463 fd_install(fd, f);
464
465 /* Event info records order is: dir fid + name, child fid */
466 if (fanotify_event_dir_fh_len(event)) {
467 info_type = info->name_len ? FAN_EVENT_INFO_TYPE_DFID_NAME :
468 FAN_EVENT_INFO_TYPE_DFID;
469 ret = copy_info_to_user(fanotify_event_fsid(event),
470 fanotify_info_dir_fh(info),
471 info_type, fanotify_info_name(info),
472 info->name_len, buf, count);
473 if (ret < 0)
474 goto out_close_fd;
475
476 buf += ret;
477 count -= ret;
478 }
479
480 if (fanotify_event_object_fh_len(event)) {
481 const char *dot = NULL;
482 int dot_len = 0;
483
484 if (fid_mode == FAN_REPORT_FID || info_type) {
485 /*
486 * With only group flag FAN_REPORT_FID only type FID is
487 * reported. Second info record type is always FID.
488 */
489 info_type = FAN_EVENT_INFO_TYPE_FID;
490 } else if ((fid_mode & FAN_REPORT_NAME) &&
491 (event->mask & FAN_ONDIR)) {
492 /*
493 * With group flag FAN_REPORT_NAME, if name was not
494 * recorded in an event on a directory, report the
495 * name "." with info type DFID_NAME.
496 */
497 info_type = FAN_EVENT_INFO_TYPE_DFID_NAME;
498 dot = ".";
499 dot_len = 1;
500 } else if ((event->mask & ALL_FSNOTIFY_DIRENT_EVENTS) ||
501 (event->mask & FAN_ONDIR)) {
502 /*
503 * With group flag FAN_REPORT_DIR_FID, a single info
504 * record has type DFID for directory entry modification
505 * event and for event on a directory.
506 */
507 info_type = FAN_EVENT_INFO_TYPE_DFID;
508 } else {
509 /*
510 * With group flags FAN_REPORT_DIR_FID|FAN_REPORT_FID,
511 * a single info record has type FID for event on a
512 * non-directory, when there is no directory to report.
513 * For example, on FAN_DELETE_SELF event.
514 */
515 info_type = FAN_EVENT_INFO_TYPE_FID;
516 }
517
518 ret = copy_info_to_user(fanotify_event_fsid(event),
519 fanotify_event_object_fh(event),
520 info_type, dot, dot_len, buf, count);
521 if (ret < 0)
522 goto out_close_fd;
^^^^^^^^^^^^^^^^^
523
524 buf += ret;
525 count -= ret;
526 }
527
528 return metadata.event_len;
529
530 out_close_fd:
531 if (fd != FAN_NOFD) {
532 put_unused_fd(fd);
--> 533 fput(f);
^^^^^^^
This leads to a NULL dereference
534 }
535 return ret;
536 }
regards,
dan carpenter
