From c3702afa8cfaa5cce5a9c751b63bf0b8fb3c986f Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Sun, 26 Apr 2020 00:25:10 -0500 Subject: [PATCH] FBSD: Use user creds for afs_vop_putpages() Currently, the FBSD afs_vop_putpages uses the credentials of the current process for writing data back to the fileserver. Usually this works, but sometimes we're being called from syncer(4), which runs as root instead of as the user accessing the relevant file. This means that instead of using the tokens from the user that wrote to the relevant file, we use whatever tokens belong to uid 0. This usually causes an EACCES error when trying to write the data back to the fileserver, causing us to store EACCES (13) in avc->vc_error, and possibly causing a message in the kernel log like so: afs: failed to store file (0/13) Since we set vc_error during these errors, this can also cause access to the file to fail for the normal user process until vc_error is cleared (such as when the file is closed). To avoid this, store the credentials of the current user that successfully opens the file for writing (in avc->cred), and use those creds for writing back data to the fileserver. This is the same approach that LINUX uses as of commit 70c8deab (Use user credentials for Linux writepage()), and the NFS client code in FreeBSD itself (see the usage of n_writecred in struct nfsnode as of 12.1-RELEASE). Change-Id: I592d709b68d746bbdb326dfd7d012d6de829b905 Reviewed-on: https://gerrit.openafs.org/14164 Tested-by: BuildBot Reviewed-by: Benjamin Kaduk --- src/afs/FBSD/osi_vnodeops.c | 27 ++++++++++++++++++++++++++- src/afs/VNOPS/afs_vnop_open.c | 2 +- src/afs/afs.h | 4 ++-- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/afs/FBSD/osi_vnodeops.c b/src/afs/FBSD/osi_vnodeops.c index 2a0c2ab64a..5c301d8e28 100644 --- a/src/afs/FBSD/osi_vnodeops.c +++ b/src/afs/FBSD/osi_vnodeops.c @@ -386,6 +386,14 @@ afs_vop_close(ap) else code = afs_close(avc, ap->a_fflag, afs_osi_credp); osi_FlushPages(avc, ap->a_cred); /* hold GLOCK, but not basic vnode lock */ + + ObtainWriteLock(&avc->lock, 808); + if (avc->cred != NULL) { + crfree(avc->cred); + avc->cred = NULL; + } + ReleaseWriteLock(&avc->lock); + AFS_GUNLOCK(); return code; } @@ -695,6 +703,7 @@ afs_vop_putpages(struct vop_putpages_args *ap) vm_offset_t kva; struct vnode *vp; struct vcache *avc; + struct ucred *cred; memset(&uio, 0, sizeof(uio)); memset(&iov, 0, sizeof(iov)); @@ -736,7 +745,22 @@ afs_vop_putpages(struct vop_putpages_args *ap) * sync |= IO_INVAL; */ AFS_GLOCK(); - code = afs_write(avc, &uio, sync, osi_curcred(), 0); + + ObtainReadLock(&avc->lock); + if (avc->cred != NULL) { + /* + * Use the creds from the process that opened this file for writing; if + * any. Otherwise, if we use the current process's creds, we may use + * the creds for uid 0 if we are writing back pages from the syncer(4) + * process. + */ + cred = crhold(avc->cred); + } else { + cred = crhold(curthread->td_ucred); + } + ReleaseReadLock(&avc->lock); + + code = afs_write(avc, &uio, sync, cred, 0); AFS_GUNLOCK(); pmap_qremove(kva, npages); @@ -751,6 +775,7 @@ afs_vop_putpages(struct vop_putpages_args *ap) } AFS_VM_OBJECT_WUNLOCK(vp->v_object); } + crfree(cred); return ap->a_rtvals[0]; } diff --git a/src/afs/VNOPS/afs_vnop_open.c b/src/afs/VNOPS/afs_vnop_open.c index fc947e8da1..43e2210764 100644 --- a/src/afs/VNOPS/afs_vnop_open.c +++ b/src/afs/VNOPS/afs_vnop_open.c @@ -151,7 +151,7 @@ afs_open(struct vcache **avcp, afs_int32 aflags, afs_ucred_t *acred) if (writing) tvc->execsOrWriters++; tvc->opens++; -#if defined(AFS_SGI_ENV) || defined (AFS_LINUX_ENV) +#if defined(AFS_SGI_ENV) || defined (AFS_LINUX_ENV) || defined(AFS_FBSD_ENV) if (writing && tvc->cred == NULL) { crhold(acred); tvc->cred = acred; diff --git a/src/afs/afs.h b/src/afs/afs.h index c83f825b8f..7d04d24472 100644 --- a/src/afs/afs.h +++ b/src/afs/afs.h @@ -969,8 +969,8 @@ struct vcache { struct bhv_desc vc_bhv_desc; /* vnode's behavior data. */ #endif #endif /* AFS_SGI_ENV */ -#if defined(AFS_LINUX_ENV) - cred_t *cred; /* last writer's cred */ +#if defined(AFS_LINUX_ENV) || defined(AFS_FBSD_ENV) + afs_ucred_t *cred; /* last writer's cred */ #endif #ifdef AFS_LINUX_ENV struct dentry *target_link; /* dentry we prefer, when we are redirecting