Yep. You can commit it or I can?
(You can put reviewed by me if you choose to commit it.)
Thanks, rick
On Thu, Jul 10, 2025 at 5:27 AM Dag-Erling Smørgrav <d...@freebsd.org> wrote:
>
> CAUTION: This email originated from outside of the University of Guelph. Do
> not click links or open attachments unless you recognize the sender and know
> the content is safe. If in doubt, forward suspicious emails to
> ith...@uoguelph.ca.
>
> Rick Macklem <rmack...@freebsd.org> writes:
> > diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
> > index 0049d7edca33..fbfcdafaa06b 100644
> > --- a/sys/fs/nfsclient/nfs_clvnops.c
> > +++ b/sys/fs/nfsclient/nfs_clvnops.c
> > [...]
> > @@ -1092,7 +1100,8 @@ nfs_setattr(struct vop_setattr_args *ap)
> > vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL ||
> > vap->va_mtime.tv_sec != VNOVAL ||
> > vap->va_birthtime.tv_sec != VNOVAL ||
> > - vap->va_mode != (mode_t)VNOVAL) &&
> > + vap->va_mode != (mode_t)VNOVAL ||
> > + vap->va_flags != (u_long)VNOVAL) &&
> > (vp->v_mount->mnt_flag & MNT_RDONLY))
> > return (EROFS);
> > if (vap->va_size != VNOVAL) {
>
> vap->va_flags was already checked (in the line just before the first
> line of context), albeit without a cast. Coverity erroneously claims
> that this causes the entire expression to always be true, because it
> thinks VNOVAL and (u_long)VNOVAL are two different values. That's not
> the case, but you probably still want this:
>
> diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvnops.c
> index fbfcdafaa06b..fa451887e73e 100644
> --- a/sys/fs/nfsclient/nfs_clvnops.c
> +++ b/sys/fs/nfsclient/nfs_clvnops.c
> @@ -1096,12 +1096,11 @@ nfs_setattr(struct vop_setattr_args *ap)
> /*
> * Disallow write attempts if the filesystem is mounted read-only.
> */
> - if ((vap->va_flags != VNOVAL || vap->va_uid != (uid_t)VNOVAL ||
> + if ((vap->va_flags != (u_long)VNOVAL || vap->va_uid != (uid_t)VNOVAL
> ||
> vap->va_gid != (gid_t)VNOVAL || vap->va_atime.tv_sec != VNOVAL ||
> vap->va_mtime.tv_sec != VNOVAL ||
> vap->va_birthtime.tv_sec != VNOVAL ||
> - vap->va_mode != (mode_t)VNOVAL ||
> - vap->va_flags != (u_long)VNOVAL) &&
> + vap->va_mode != (mode_t)VNOVAL) &&
> (vp->v_mount->mnt_flag & MNT_RDONLY))
> return (EROFS);
> if (vap->va_size != VNOVAL) {
>
> DES
> --
> Dag-Erling Smørgrav - d...@freebsd.org
