[bug report] fanotify: fix copy_event_to_user() fid error clean up

[Dan Carpenter <dan.carpenter@oracle.com>]

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