* [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion
@ 2025-09-23 17:08 Darrick J. Wong
2025-09-23 17:10 ` [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr Darrick J. Wong
2025-09-24 9:03 ` [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Andrey Albershteyn
0 siblings, 2 replies; 5+ messages in thread
From: Darrick J. Wong @ 2025-09-23 17:08 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: xfs
From: Darrick J. Wong <djwong@kernel.org>
libxfs functions return negative errno, so utilities must invert the
return values from such functions. Caught by xfs/437.
Fixes: 8a4ea72724930c ("proto: add ability to populate a filesystem from a directory")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
mkfs/proto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mkfs/proto.c b/mkfs/proto.c
index bfeeb5ac638185..2b29240db95f74 100644
--- a/mkfs/proto.c
+++ b/mkfs/proto.c
@@ -1425,7 +1425,7 @@ handle_hardlink(
if (dst_ino == 0)
return false;
- error = libxfs_iget(mp, NULL, dst_ino, 0, &ip);
+ error = -libxfs_iget(mp, NULL, dst_ino, 0, &ip);
if (error)
fail(_("failed to get inode"), error);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr
2025-09-23 17:08 [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Darrick J. Wong
@ 2025-09-23 17:10 ` Darrick J. Wong
2025-09-24 13:18 ` Andrey Albershteyn
2025-09-24 9:03 ` [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Andrey Albershteyn
1 sibling, 1 reply; 5+ messages in thread
From: Darrick J. Wong @ 2025-09-23 17:10 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: xfs
From: Darrick J. Wong <djwong@kernel.org>
xfs/633 crashes rdump_fileattrs_path passes a NULL struct stat pointer
and then the fallback code dereferences it to get the file mode.
Instead, let's just pass the stat mode directly to it, because that's
the only piece of information that it needs.
Fixes: 128ac4dadbd633 ("xfs_db: use file_setattr to copy attributes on special files with rdump")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
libfrog/file_attr.h | 9 ++-------
db/rdump.c | 4 ++--
io/attr.c | 4 ++--
libfrog/file_attr.c | 4 ++--
quota/project.c | 6 ++++--
5 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h
index df9b6181d52cf9..2a1c0d42d0a771 100644
--- a/libfrog/file_attr.h
+++ b/libfrog/file_attr.h
@@ -24,12 +24,7 @@ xfrog_file_getattr(
struct file_attr *fa,
const unsigned int at_flags);
-int
-xfrog_file_setattr(
- const int dfd,
- const char *path,
- const struct stat *stat,
- struct file_attr *fa,
- const unsigned int at_flags);
+int xfrog_file_setattr(const int dfd, const char *path, const mode_t mode,
+ struct file_attr *fa, const unsigned int at_flags);
#endif /* __LIBFROG_FILE_ATTR_H__ */
diff --git a/db/rdump.c b/db/rdump.c
index 84ca3156d60598..26f9babad62be1 100644
--- a/db/rdump.c
+++ b/db/rdump.c
@@ -188,8 +188,8 @@ rdump_fileattrs_path(
return 1;
}
- ret = xfrog_file_setattr(destdir->fd, pbuf->path, NULL, &fa,
- AT_SYMLINK_NOFOLLOW);
+ ret = xfrog_file_setattr(destdir->fd, pbuf->path, VFS_I(ip)->i_mode,
+ &fa, AT_SYMLINK_NOFOLLOW);
if (ret) {
if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY)
lost_mask |= LOST_FSXATTR;
diff --git a/io/attr.c b/io/attr.c
index 022ca5f1df1b7c..9563ff74e44777 100644
--- a/io/attr.c
+++ b/io/attr.c
@@ -261,7 +261,7 @@ chattr_callback(
attr.fa_xflags |= orflags;
attr.fa_xflags &= ~andflags;
- error = xfrog_file_setattr(AT_FDCWD, path, stat, &attr,
+ error = xfrog_file_setattr(AT_FDCWD, path, stat->st_mode, &attr,
AT_SYMLINK_NOFOLLOW);
if (error) {
fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
@@ -357,7 +357,7 @@ chattr_f(
attr.fa_xflags |= orflags;
attr.fa_xflags &= ~andflags;
- error = xfrog_file_setattr(AT_FDCWD, name, &st, &attr,
+ error = xfrog_file_setattr(AT_FDCWD, name, st.st_mode, &attr,
AT_SYMLINK_NOFOLLOW);
if (error) {
fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c
index bb51ac6eb2ef95..c2cbcb4e14659c 100644
--- a/libfrog/file_attr.c
+++ b/libfrog/file_attr.c
@@ -85,7 +85,7 @@ int
xfrog_file_setattr(
const int dfd,
const char *path,
- const struct stat *stat,
+ const mode_t mode,
struct file_attr *fa,
const unsigned int at_flags)
{
@@ -103,7 +103,7 @@ xfrog_file_setattr(
return error;
#endif
- if (SPECIAL_FILE(stat->st_mode)) {
+ if (SPECIAL_FILE(mode)) {
errno = EOPNOTSUPP;
return -1;
}
diff --git a/quota/project.c b/quota/project.c
index 5832e1474e2549..33449e01ef4dbb 100644
--- a/quota/project.c
+++ b/quota/project.c
@@ -157,7 +157,8 @@ clear_project(
fa.fa_projid = 0;
fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT;
- error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
+ error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
+ AT_SYMLINK_NOFOLLOW);
if (error) {
fprintf(stderr, _("%s: cannot clear project on %s: %s\n"),
progname, path, strerror(errno));
@@ -205,7 +206,8 @@ setup_project(
if (S_ISDIR(stat->st_mode))
fa.fa_xflags |= FS_XFLAG_PROJINHERIT;
- error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
+ error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
+ AT_SYMLINK_NOFOLLOW);
if (error) {
fprintf(stderr, _("%s: cannot set project on %s: %s\n"),
progname, path, strerror(errno));
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion
2025-09-23 17:08 [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Darrick J. Wong
2025-09-23 17:10 ` [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr Darrick J. Wong
@ 2025-09-24 9:03 ` Andrey Albershteyn
1 sibling, 0 replies; 5+ messages in thread
From: Andrey Albershteyn @ 2025-09-24 9:03 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On 2025-09-23 10:08:57, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> libxfs functions return negative errno, so utilities must invert the
> return values from such functions. Caught by xfs/437.
>
> Fixes: 8a4ea72724930c ("proto: add ability to populate a filesystem from a directory")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
lgtm
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
> ---
> mkfs/proto.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index bfeeb5ac638185..2b29240db95f74 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -1425,7 +1425,7 @@ handle_hardlink(
> if (dst_ino == 0)
> return false;
>
> - error = libxfs_iget(mp, NULL, dst_ino, 0, &ip);
> + error = -libxfs_iget(mp, NULL, dst_ino, 0, &ip);
> if (error)
> fail(_("failed to get inode"), error);
>
>
--
- Andrey
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr
2025-09-23 17:10 ` [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr Darrick J. Wong
@ 2025-09-24 13:18 ` Andrey Albershteyn
2025-09-24 21:52 ` Darrick J. Wong
0 siblings, 1 reply; 5+ messages in thread
From: Andrey Albershteyn @ 2025-09-24 13:18 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On 2025-09-23 10:10:27, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> xfs/633 crashes rdump_fileattrs_path passes a NULL struct stat pointer
> and then the fallback code dereferences it to get the file mode.
Oh is it latest xfsprogs with older kernel (without file_[g]etattr)?
(I see this on 6.16)
> Instead, let's just pass the stat mode directly to it, because that's
> the only piece of information that it needs.
>
> Fixes: 128ac4dadbd633 ("xfs_db: use file_setattr to copy attributes on special files with rdump")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> libfrog/file_attr.h | 9 ++-------
> db/rdump.c | 4 ++--
> io/attr.c | 4 ++--
> libfrog/file_attr.c | 4 ++--
> quota/project.c | 6 ++++--
> 5 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h
> index df9b6181d52cf9..2a1c0d42d0a771 100644
> --- a/libfrog/file_attr.h
> +++ b/libfrog/file_attr.h
> @@ -24,12 +24,7 @@ xfrog_file_getattr(
> struct file_attr *fa,
> const unsigned int at_flags);
>
> -int
> -xfrog_file_setattr(
> - const int dfd,
> - const char *path,
> - const struct stat *stat,
> - struct file_attr *fa,
> - const unsigned int at_flags);
> +int xfrog_file_setattr(const int dfd, const char *path, const mode_t mode,
> + struct file_attr *fa, const unsigned int at_flags);
Is this formatting change intentional? (maybe then the
xfrog_file_getattr also)
otherwise lgtm
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
>
> #endif /* __LIBFROG_FILE_ATTR_H__ */
> diff --git a/db/rdump.c b/db/rdump.c
> index 84ca3156d60598..26f9babad62be1 100644
> --- a/db/rdump.c
> +++ b/db/rdump.c
> @@ -188,8 +188,8 @@ rdump_fileattrs_path(
> return 1;
> }
>
> - ret = xfrog_file_setattr(destdir->fd, pbuf->path, NULL, &fa,
> - AT_SYMLINK_NOFOLLOW);
> + ret = xfrog_file_setattr(destdir->fd, pbuf->path, VFS_I(ip)->i_mode,
> + &fa, AT_SYMLINK_NOFOLLOW);
> if (ret) {
> if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY)
> lost_mask |= LOST_FSXATTR;
> diff --git a/io/attr.c b/io/attr.c
> index 022ca5f1df1b7c..9563ff74e44777 100644
> --- a/io/attr.c
> +++ b/io/attr.c
> @@ -261,7 +261,7 @@ chattr_callback(
>
> attr.fa_xflags |= orflags;
> attr.fa_xflags &= ~andflags;
> - error = xfrog_file_setattr(AT_FDCWD, path, stat, &attr,
> + error = xfrog_file_setattr(AT_FDCWD, path, stat->st_mode, &attr,
> AT_SYMLINK_NOFOLLOW);
> if (error) {
> fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
> @@ -357,7 +357,7 @@ chattr_f(
>
> attr.fa_xflags |= orflags;
> attr.fa_xflags &= ~andflags;
> - error = xfrog_file_setattr(AT_FDCWD, name, &st, &attr,
> + error = xfrog_file_setattr(AT_FDCWD, name, st.st_mode, &attr,
> AT_SYMLINK_NOFOLLOW);
> if (error) {
> fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
> diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c
> index bb51ac6eb2ef95..c2cbcb4e14659c 100644
> --- a/libfrog/file_attr.c
> +++ b/libfrog/file_attr.c
> @@ -85,7 +85,7 @@ int
> xfrog_file_setattr(
> const int dfd,
> const char *path,
> - const struct stat *stat,
> + const mode_t mode,
> struct file_attr *fa,
> const unsigned int at_flags)
> {
> @@ -103,7 +103,7 @@ xfrog_file_setattr(
> return error;
> #endif
>
> - if (SPECIAL_FILE(stat->st_mode)) {
> + if (SPECIAL_FILE(mode)) {
> errno = EOPNOTSUPP;
> return -1;
> }
> diff --git a/quota/project.c b/quota/project.c
> index 5832e1474e2549..33449e01ef4dbb 100644
> --- a/quota/project.c
> +++ b/quota/project.c
> @@ -157,7 +157,8 @@ clear_project(
> fa.fa_projid = 0;
> fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT;
>
> - error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
> + error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
> + AT_SYMLINK_NOFOLLOW);
> if (error) {
> fprintf(stderr, _("%s: cannot clear project on %s: %s\n"),
> progname, path, strerror(errno));
> @@ -205,7 +206,8 @@ setup_project(
> if (S_ISDIR(stat->st_mode))
> fa.fa_xflags |= FS_XFLAG_PROJINHERIT;
>
> - error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
> + error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
> + AT_SYMLINK_NOFOLLOW);
> if (error) {
> fprintf(stderr, _("%s: cannot set project on %s: %s\n"),
> progname, path, strerror(errno));
>
--
- Andrey
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr
2025-09-24 13:18 ` Andrey Albershteyn
@ 2025-09-24 21:52 ` Darrick J. Wong
0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2025-09-24 21:52 UTC (permalink / raw)
To: Andrey Albershteyn; +Cc: xfs
On Wed, Sep 24, 2025 at 03:18:15PM +0200, Andrey Albershteyn wrote:
> On 2025-09-23 10:10:27, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > xfs/633 crashes rdump_fileattrs_path passes a NULL struct stat pointer
> > and then the fallback code dereferences it to get the file mode.
>
> Oh is it latest xfsprogs with older kernel (without file_[g]etattr)?
>
> (I see this on 6.16)
Yes, the crash happens on 6.16-rc7 where there is no file_setattr call,
and does not happen on for-next where file_setattr does exist.
> > Instead, let's just pass the stat mode directly to it, because that's
> > the only piece of information that it needs.
> >
> > Fixes: 128ac4dadbd633 ("xfs_db: use file_setattr to copy attributes on special files with rdump")
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> > libfrog/file_attr.h | 9 ++-------
> > db/rdump.c | 4 ++--
> > io/attr.c | 4 ++--
> > libfrog/file_attr.c | 4 ++--
> > quota/project.c | 6 ++++--
> > 5 files changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/libfrog/file_attr.h b/libfrog/file_attr.h
> > index df9b6181d52cf9..2a1c0d42d0a771 100644
> > --- a/libfrog/file_attr.h
> > +++ b/libfrog/file_attr.h
> > @@ -24,12 +24,7 @@ xfrog_file_getattr(
> > struct file_attr *fa,
> > const unsigned int at_flags);
> >
> > -int
> > -xfrog_file_setattr(
> > - const int dfd,
> > - const char *path,
> > - const struct stat *stat,
> > - struct file_attr *fa,
> > - const unsigned int at_flags);
> > +int xfrog_file_setattr(const int dfd, const char *path, const mode_t mode,
> > + struct file_attr *fa, const unsigned int at_flags);
>
> Is this formatting change intentional? (maybe then the
> xfrog_file_getattr also)
Yes, compressed is the usual style for function declarations.
It's only the definition that gets the expanded format.
(I didn't hassle you about it when you submitted the original patch
because I'm trying to stop doing that to people ;))
> otherwise lgtm
> Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
Thanks!
--D
> >
> > #endif /* __LIBFROG_FILE_ATTR_H__ */
> > diff --git a/db/rdump.c b/db/rdump.c
> > index 84ca3156d60598..26f9babad62be1 100644
> > --- a/db/rdump.c
> > +++ b/db/rdump.c
> > @@ -188,8 +188,8 @@ rdump_fileattrs_path(
> > return 1;
> > }
> >
> > - ret = xfrog_file_setattr(destdir->fd, pbuf->path, NULL, &fa,
> > - AT_SYMLINK_NOFOLLOW);
> > + ret = xfrog_file_setattr(destdir->fd, pbuf->path, VFS_I(ip)->i_mode,
> > + &fa, AT_SYMLINK_NOFOLLOW);
> > if (ret) {
> > if (errno == EOPNOTSUPP || errno == EPERM || errno == ENOTTY)
> > lost_mask |= LOST_FSXATTR;
> > diff --git a/io/attr.c b/io/attr.c
> > index 022ca5f1df1b7c..9563ff74e44777 100644
> > --- a/io/attr.c
> > +++ b/io/attr.c
> > @@ -261,7 +261,7 @@ chattr_callback(
> >
> > attr.fa_xflags |= orflags;
> > attr.fa_xflags &= ~andflags;
> > - error = xfrog_file_setattr(AT_FDCWD, path, stat, &attr,
> > + error = xfrog_file_setattr(AT_FDCWD, path, stat->st_mode, &attr,
> > AT_SYMLINK_NOFOLLOW);
> > if (error) {
> > fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
> > @@ -357,7 +357,7 @@ chattr_f(
> >
> > attr.fa_xflags |= orflags;
> > attr.fa_xflags &= ~andflags;
> > - error = xfrog_file_setattr(AT_FDCWD, name, &st, &attr,
> > + error = xfrog_file_setattr(AT_FDCWD, name, st.st_mode, &attr,
> > AT_SYMLINK_NOFOLLOW);
> > if (error) {
> > fprintf(stderr, _("%s: cannot set flags on %s: %s\n"),
> > diff --git a/libfrog/file_attr.c b/libfrog/file_attr.c
> > index bb51ac6eb2ef95..c2cbcb4e14659c 100644
> > --- a/libfrog/file_attr.c
> > +++ b/libfrog/file_attr.c
> > @@ -85,7 +85,7 @@ int
> > xfrog_file_setattr(
> > const int dfd,
> > const char *path,
> > - const struct stat *stat,
> > + const mode_t mode,
> > struct file_attr *fa,
> > const unsigned int at_flags)
> > {
> > @@ -103,7 +103,7 @@ xfrog_file_setattr(
> > return error;
> > #endif
> >
> > - if (SPECIAL_FILE(stat->st_mode)) {
> > + if (SPECIAL_FILE(mode)) {
> > errno = EOPNOTSUPP;
> > return -1;
> > }
> > diff --git a/quota/project.c b/quota/project.c
> > index 5832e1474e2549..33449e01ef4dbb 100644
> > --- a/quota/project.c
> > +++ b/quota/project.c
> > @@ -157,7 +157,8 @@ clear_project(
> > fa.fa_projid = 0;
> > fa.fa_xflags &= ~FS_XFLAG_PROJINHERIT;
> >
> > - error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
> > + error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
> > + AT_SYMLINK_NOFOLLOW);
> > if (error) {
> > fprintf(stderr, _("%s: cannot clear project on %s: %s\n"),
> > progname, path, strerror(errno));
> > @@ -205,7 +206,8 @@ setup_project(
> > if (S_ISDIR(stat->st_mode))
> > fa.fa_xflags |= FS_XFLAG_PROJINHERIT;
> >
> > - error = xfrog_file_setattr(dfd, path, stat, &fa, AT_SYMLINK_NOFOLLOW);
> > + error = xfrog_file_setattr(dfd, path, stat->st_mode, &fa,
> > + AT_SYMLINK_NOFOLLOW);
> > if (error) {
> > fprintf(stderr, _("%s: cannot set project on %s: %s\n"),
> > progname, path, strerror(errno));
> >
>
> --
> - Andrey
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-09-24 21:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-23 17:08 [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Darrick J. Wong
2025-09-23 17:10 ` [PATCH 2/2] libfrog: pass mode to xfrog_file_setattr Darrick J. Wong
2025-09-24 13:18 ` Andrey Albershteyn
2025-09-24 21:52 ` Darrick J. Wong
2025-09-24 9:03 ` [PATCH 1/2] mkfs: fix libxfs_iget return value sign inversion Andrey Albershteyn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).