Date: Thu, 10 Jul 2025 07:09:10 -0700 From: Rick Macklem <rick.macklem@gmail.com> To: =?UTF-8?Q?Dag=2DErling_Sm=C3=B8rgrav?= <des@freebsd.org> Cc: Rick Macklem <rmacklem@freebsd.org>, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: c5d72d29fe0e - main - nfsv4: Add support for the NFSv4 hidden and system attributes Message-ID: <CAM5tNy5=7nPzm9sDJayy63kv-SWVXjjrKnktSgrrgT=6aOv4yA@mail.gmail.com> In-Reply-To: <867c0gcjt3.fsf@ltc.des.dev> References: <202507062254.566MsbnL090918@gitrepo.freebsd.org> <867c0gcjt3.fsf@ltc.des.dev>
next in thread | previous in thread | raw e-mail | index | archive | help
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=E2=80=AFAM Dag-Erling Sm=C3=B8rgrav <des@freeb=
sd.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 IThelp@=
uoguelph.ca.
>
> Rick Macklem <rmacklem@FreeBSD.org> writes:
> > diff --git a/sys/fs/nfsclient/nfs_clvnops.c b/sys/fs/nfsclient/nfs_clvn=
ops.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 !=3D (gid_t)VNOVAL || vap->va_atime.tv_sec !=3D V=
NOVAL ||
> > vap->va_mtime.tv_sec !=3D VNOVAL ||
> > vap->va_birthtime.tv_sec !=3D VNOVAL ||
> > - vap->va_mode !=3D (mode_t)VNOVAL) &&
> > + vap->va_mode !=3D (mode_t)VNOVAL ||
> > + vap->va_flags !=3D (u_long)VNOVAL) &&
> > (vp->v_mount->mnt_flag & MNT_RDONLY))
> > return (EROFS);
> > if (vap->va_size !=3D 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_clvnop=
s.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 !=3D VNOVAL || vap->va_uid !=3D (uid_t)VNOVAL =
||
> + if ((vap->va_flags !=3D (u_long)VNOVAL || vap->va_uid !=3D (uid_t=
)VNOVAL ||
> vap->va_gid !=3D (gid_t)VNOVAL || vap->va_atime.tv_sec !=3D V=
NOVAL ||
> vap->va_mtime.tv_sec !=3D VNOVAL ||
> vap->va_birthtime.tv_sec !=3D VNOVAL ||
> - vap->va_mode !=3D (mode_t)VNOVAL ||
> - vap->va_flags !=3D (u_long)VNOVAL) &&
> + vap->va_mode !=3D (mode_t)VNOVAL) &&
> (vp->v_mount->mnt_flag & MNT_RDONLY))
> return (EROFS);
> if (vap->va_size !=3D VNOVAL) {
>
> DES
> --
> Dag-Erling Sm=C3=B8rgrav - des@FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAM5tNy5=7nPzm9sDJayy63kv-SWVXjjrKnktSgrrgT=6aOv4yA>