On Mon, Jul 28, 2025 at 08:57:21PM +0000, Konstantin Belousov wrote: > The branch main has been updated by kib: > > URL: > https://cgit.FreeBSD.org/src/commit/?id=610319c766e941de96e52f2d28fea9f8cfc51aeb > > commit 610319c766e941de96e52f2d28fea9f8cfc51aeb > Author: Konstantin Belousov <k...@freebsd.org> > AuthorDate: 2025-07-27 13:50:57 +0000 > Commit: Konstantin Belousov <k...@freebsd.org> > CommitDate: 2025-07-28 20:57:14 +0000 > > ufs_vnops.c: newparent is not bool > > Use proper comparision operators when we need to see if newparent was > set to not-zero value. > > Reviewed by: mckusick, olce > Sponsored by: The FreeBSD Foundation > MFC after: 1 week > Differential revision: https://reviews.freebsd.org/D51573 > --- > sys/ufs/ufs/ufs_vnops.c | 15 +++++++-------- > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c > index 406b8f943077..2757fb066981 100644 > --- a/sys/ufs/ufs/ufs_vnops.c > +++ b/sys/ufs/ufs/ufs_vnops.c > @@ -1476,7 +1476,7 @@ relock: > * the user must have write permission in the source so > * as to be able to change "..". > */ > - if (doingdirectory && newparent) { > + if (doingdirectory && newparent != 0) { > error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred, curthread); > if (error) > goto unlockout; > @@ -1539,7 +1539,7 @@ relock: > if (tip == NULL) { > if (ITODEV(tdp) != ITODEV(fip)) > panic("ufs_rename: EXDEV"); > - if (doingdirectory && newparent) { > + if (doingdirectory && newparent != 0) { > /* > * Account for ".." in new directory. > * When source and destination have the same > @@ -1632,7 +1632,7 @@ relock: > goto bad; > } > if (doingdirectory) { > - if (!newparent) { > + if (newparent == 0) { > tdp->i_effnlink--; > if (DOINGSOFTDEP(tdvp)) > softdep_change_linkcnt(tdp); > @@ -1642,11 +1642,10 @@ relock: > softdep_change_linkcnt(tip); > } > error = ufs_dirrewrite(tdp, tip, fip->i_number, > - IFTODT(fip->i_mode), > - (doingdirectory && newparent) ? newparent : doingdirectory); > + IFTODT(fip->i_mode), doingdirectory);
Is this part of the change correct? syzbot is reporting some panics after this change and commit c069ca085bd185eda4a90dc4bc2b76cceb74579d: https://syzkaller.appspot.com/bug?extid=18722c8e4008048efb51 https://syzkaller.appspot.com/bug?extid=6a4ea1e13f4e07369785 https://syzkaller.appspot.com/bug?extid=5cb82352555d5d505640 https://syzkaller.appspot.com/bug?extid=602fb6ee1a39abfd3b5c https://syzkaller.appspot.com/bug?extid=fb35cce6a6f5075a6692 https://syzkaller.appspot.com/bug?extid=6fb8cb919cc686d1a1d0 https://syzkaller.appspot.com/bug?extid=98c39c45a437812f7683 https://syzkaller.appspot.com/bug?extid=02cb048d48b51bcd9c41 > if (error) { > if (doingdirectory) { > - if (!newparent) { > + if (newparent == 0) { > tdp->i_effnlink++; > if (DOINGSOFTDEP(tdvp)) > softdep_change_linkcnt(tdp); > @@ -1669,7 +1668,7 @@ relock: > * disk, so when running with that code we avoid doing > * them now. > */ > - if (!newparent) { > + if (newparent == 0) { > tdp->i_nlink--; > DIP_SET_NLINK(tdp, tdp->i_nlink); > UFS_INODE_SET_FLAG(tdp, IN_CHANGE); > @@ -1698,7 +1697,7 @@ relock: > * parent directory must be decremented > * and ".." set to point to the new parent. > */ > - if (doingdirectory && newparent) { > + if (doingdirectory && newparent != 0) { > /* > * Set the directory depth based on its new parent. > */
