statfs: add missing f_flags assignment

Message ID 87r1q4t2sn.wl-chenli@uniontech.com
State Committed
Commit d3a5ae6ad16ba488dec7d15c6554585d9a405336
Headers
Series statfs: add missing f_flags assignment |

Commit Message

Chen Li Oct. 12, 2020, 5:46 a.m. UTC
  f_flags is added into struct statfs since Linux 2.6.36, which is lacked
in glibc's statfs64.c until now. So mount flags is uninitialized on
platforms having no statfs64 syscall in kernel, e.g., alpha and its derivation

Signed-off-by: chenli <chenli@uniontech.com>
---
 sysdeps/unix/sysv/linux/statfs64.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Florian Weimer Oct. 14, 2020, 8:01 a.m. UTC | #1
* Chen Li:

> f_flags is added into struct statfs since Linux 2.6.36, which is lacked
> in glibc's statfs64.c until now. So mount flags is uninitialized on
> platforms having no statfs64 syscall in kernel, e.g., alpha and its derivation
>
> Signed-off-by: chenli <chenli@uniontech.com>
> ---
>  sysdeps/unix/sysv/linux/statfs64.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/sysdeps/unix/sysv/linux/statfs64.c b/sysdeps/unix/sysv/linux/statfs64.c
> index c941128637..2c293badc8 100644
> --- a/sysdeps/unix/sysv/linux/statfs64.c
> +++ b/sysdeps/unix/sysv/linux/statfs64.c
> @@ -78,6 +78,7 @@ __statfs64 (const char *file, struct statfs64 *buf)
>    buf->f_fsid = buf32.f_fsid;
>    buf->f_namelen = buf32.f_namelen;
>    buf->f_frsize = buf32.f_frsize;
> +  buf->f_flags = buf32.f_flags;
>    memcpy (buf->f_spare, buf32.f_spare, sizeof (buf32.f_spare));
>  
>    return 0;

I've checked that this builds on all architectures.

Does this result in a user-visible bug on some architectures besides
alpha?

May we drop the “Signed-off-by: chenli <chenli@uniontech.com>” line?
glibc does not use DCO <https://developercertificate.org/>, but due to
the size of the change, no additional paperwork is required for this
contribution.

Thanks,
Florian
  
Chen Li Oct. 15, 2020, 9 a.m. UTC | #2
> Does this result in a user-visible bug on some architectures besides alpha?

all arch have no statfs64 syscall support should suffer from this bug. I search
my 4.4 kernel, and found "alpha arc c6x h8300 hexagon metag nios2 openrisc score
tile um unicore32 x86" all don't have this syscall.

AFAIK, some uses of statvfs will also take use of statfs64 internally, e.g.,
systemd's Protect*:

static int get_mount_flags(const char *path, unsigned long *flags) {
        struct statvfs buf;

        if (statvfs(path, &buf) < 0)
                return -errno;
        *flags = buf.f_flag;
        return 0;
}

So, on architectures I mentioned above, this bug may also cause some secure
services fail to start due to uninitialized f_flags.

> May we drop the “Signed-off-by: chenli <chenli@uniontech.com>” line?
> glibc does not use DCO <https://developercertificate.org/>, but due to
> the size of the change, no additional paperwork is required for this
> contribution.

Of course, feel free to remove this line. Also, please tell me if I should
remove this line and reposted this patch by myself, thanks.
  
Florian Weimer Oct. 15, 2020, 9:42 a.m. UTC | #3
* Chen Li:

>> Does this result in a user-visible bug on some architectures besides alpha?
>
> all arch have no statfs64 syscall support should suffer from this
> bug. I search my 4.4 kernel, and found "alpha arc c6x h8300 hexagon
> metag nios2 openrisc score tile um unicore32 x86" all don't have this
> syscall.

Hmm, I think everything except alpha has either a 64-bit statfs, or
statfs64.  So I'm not bothering with adding a test case or filing a bug.

> Of course, feel free to remove this line. Also, please tell me if I should
> remove this line and reposted this patch by myself, thanks.

Thanks, I've pushed your patch.

Florian
  

Patch

diff --git a/sysdeps/unix/sysv/linux/statfs64.c b/sysdeps/unix/sysv/linux/statfs64.c
index c941128637..2c293badc8 100644
--- a/sysdeps/unix/sysv/linux/statfs64.c
+++ b/sysdeps/unix/sysv/linux/statfs64.c
@@ -78,6 +78,7 @@  __statfs64 (const char *file, struct statfs64 *buf)
   buf->f_fsid = buf32.f_fsid;
   buf->f_namelen = buf32.f_namelen;
   buf->f_frsize = buf32.f_frsize;
+  buf->f_flags = buf32.f_flags;
   memcpy (buf->f_spare, buf32.f_spare, sizeof (buf32.f_spare));
 
   return 0;