file: Make fget*() and getvnode*() consistent about initializing *fpp

Most fget*() functions initialize the output parameter to NULL.  Make
the externally visible interface behave consistently, and make
fget_unlocked_seq() private to kern_descrip.c.

This fixes at least one bug in a consumer, _filemon_wrapper_openat(),
which assumes that getvnode() sets the output file pointer to NULL upon
an error.

Reported by:	syzbot+01c0459408f896a5933a@syzkaller.appspotmail.com
Reviewed by:	kib
MFC after:	1 week
Sponsored by:	The FreeBSD Foundation
Differential Revision:	https://reviews.freebsd.org/D34190
This commit is contained in:
Mark Johnston 2022-02-08 12:34:20 -05:00
parent 927aa5fefd
commit 300cfb96fc
3 changed files with 12 additions and 5 deletions

View File

@ -115,6 +115,8 @@ static void fdgrowtable(struct filedesc *fdp, int nfd);
static void fdgrowtable_exp(struct filedesc *fdp, int nfd);
static void fdunused(struct filedesc *fdp, int fd);
static void fdused(struct filedesc *fdp, int fd);
static int fget_unlocked_seq(struct filedesc *fdp, int fd,
cap_rights_t *needrightsp, struct file **fpp, seqc_t *seqp);
static int getmaxfd(struct thread *td);
static u_long *filecaps_copy_prep(const struct filecaps *src);
static void filecaps_copy_finish(const struct filecaps *src,
@ -2843,6 +2845,7 @@ fget_cap_locked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
FILEDESC_LOCK_ASSERT(fdp);
*fpp = NULL;
fde = fdeget_locked(fdp, fd);
if (fde == NULL) {
error = EBADF;
@ -3014,7 +3017,7 @@ fgetvp_lookup_smr(int fd, struct nameidata *ndp, struct vnode **vpp, bool *fsear
}
#endif
int
static int
fget_unlocked_seq(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
struct file **fpp, seqc_t *seqp)
{
@ -3112,8 +3115,10 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
#endif
fdt = fdp->fd_files;
if (__predict_false((u_int)fd >= fdt->fdt_nfiles))
if (__predict_false((u_int)fd >= fdt->fdt_nfiles)) {
*fpp = NULL;
return (EBADF);
}
#ifdef CAPABILITIES
seq = seqc_read_notmodify(fd_seqc(fdt, fd));
fde = &fdt->fdt_ofiles[fd];
@ -3148,6 +3153,7 @@ fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
out_fdrop:
fdrop(fp, curthread);
out_fallback:
*fpp = NULL;
return (fget_unlocked_seq(fdp, fd, needrightsp, fpp, NULL));
}
@ -3173,6 +3179,7 @@ fget_only_user(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
MPASS(FILEDESC_IS_ONLY_USER(fdp));
*fpp = NULL;
if (__predict_false(fd >= fdp->fd_nfiles))
return (EBADF);
@ -3198,6 +3205,7 @@ fget_only_user(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
MPASS(FILEDESC_IS_ONLY_USER(fdp));
*fpp = NULL;
if (__predict_false(fd >= fdp->fd_nfiles))
return (EBADF);

View File

@ -4336,6 +4336,7 @@ getvnode_path(struct thread *td, int fd, cap_rights_t *rightsp,
*/
if (__predict_false(fp->f_vnode == NULL || fp->f_ops == &badfileops)) {
fdrop(fp, td);
*fpp = NULL;
return (EINVAL);
}
@ -4363,6 +4364,7 @@ getvnode(struct thread *td, int fd, cap_rights_t *rightsp, struct file **fpp)
*/
if (__predict_false((*fpp)->f_ops == &path_fileops)) {
fdrop(*fpp, td);
*fpp = NULL;
error = EBADF;
}

View File

@ -269,10 +269,7 @@ int fget_cap_locked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
struct file **fpp, struct filecaps *havecapsp);
int fget_cap(struct thread *td, int fd, cap_rights_t *needrightsp,
struct file **fpp, struct filecaps *havecapsp);
/* Return a referenced file from an unlocked descriptor. */
int fget_unlocked_seq(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
struct file **fpp, seqc_t *seqp);
int fget_unlocked(struct filedesc *fdp, int fd, cap_rights_t *needrightsp,
struct file **fpp);
/* Return a file pointer without a ref. FILEDESC_IS_ONLY_USER must be true. */