libmount-Fix-regression-when-mounting-with-atime.patch 5.2 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134
  1. From 2b99ee2526ae61be761b0e31c50e106dbec5e9e4 Mon Sep 17 00:00:00 2001
  2. From: Filipe Manana <fdmanana@kernel.org>
  3. Date: Thu, 17 Aug 2023 10:20:13 +0100
  4. Subject: [PATCH] libmount: Fix regression when mounting with atime
  5. A regression was introduced in v2.39 that causes mounting with the atime
  6. option to fail:
  7. $ mkfs.ext4 -F /dev/sdi
  8. $ mount -o atime /dev/sdi /mnt/sdi
  9. mount: /mnt/sdi: not mount point or bad option.
  10. dmesg(1) may have more information after failed mount system call.
  11. The failure comes from the mount_setattr(2) call returning -EINVAL. This
  12. is because we pass an invalid value for the attr_clr argument. From a
  13. strace capture we have:
  14. mount_setattr(4, "", AT_EMPTY_PATH, {attr_set=0, attr_clr=MOUNT_ATTR_NOATIME, propagation=0 /* MS_??? */, userns_fd=0}, 32) = -1 EINVAL (Invalid argument)
  15. We can't pass MOUNT_ATTR_NOATIME to mount_setattr(2) through the attr_clr
  16. argument because all atime options are exclusive, so in order to set atime
  17. one has to pass MOUNT_ATTR__ATIME to attr_clr and leave attr_set as
  18. MOUNT_ATTR_RELATIME (which is defined as a value of 0).
  19. This can be read from the man page for mount_setattr(2) and also from the
  20. kernel source:
  21. $ cat fs/namespace.c
  22. static int build_mount_kattr(const struct mount_attr *attr, size_t usize,
  23. struct mount_kattr *kattr, unsigned int flags)
  24. {
  25. (...)
  26. /*
  27. * Since the MOUNT_ATTR_<atime> values are an enum, not a bitmap,
  28. * users wanting to transition to a different atime setting cannot
  29. * simply specify the atime setting in @attr_set, but must also
  30. * specify MOUNT_ATTR__ATIME in the @attr_clr field.
  31. * So ensure that MOUNT_ATTR__ATIME can't be partially set in
  32. * @attr_clr and that @attr_set can't have any atime bits set if
  33. * MOUNT_ATTR__ATIME isn't set in @attr_clr.
  34. */
  35. if (attr->attr_clr & MOUNT_ATTR__ATIME) {
  36. if ((attr->attr_clr & MOUNT_ATTR__ATIME) != MOUNT_ATTR__ATIME)
  37. return -EINVAL;
  38. /*
  39. * Clear all previous time settings as they are mutually
  40. * exclusive.
  41. */
  42. kattr->attr_clr |= MNT_RELATIME | MNT_NOATIME;
  43. switch (attr->attr_set & MOUNT_ATTR__ATIME) {
  44. case MOUNT_ATTR_RELATIME:
  45. kattr->attr_set |= MNT_RELATIME;
  46. break;
  47. case MOUNT_ATTR_NOATIME:
  48. kattr->attr_set |= MNT_NOATIME;
  49. break;
  50. case MOUNT_ATTR_STRICTATIME:
  51. break;
  52. default:
  53. return -EINVAL;
  54. }
  55. (...)
  56. So fix this by setting attr_clr MOUNT_ATTR__ATIME if we want to clear any
  57. atime related option.
  58. Signed-off-by: Filipe Manana <fdmanana@kernel.org>
  59. ---
  60. libmount/src/optlist.c | 13 ++++++++++++-
  61. tests/expected/libmount/context-mount-flags | 3 +++
  62. tests/ts/libmount/context | 9 ++++++++-
  63. 3 files changed, 23 insertions(+), 2 deletions(-)
  64. diff --git a/libmount/src/optlist.c b/libmount/src/optlist.c
  65. index e93810b47..d0afc94f7 100644
  66. --- a/libmount/src/optlist.c
  67. +++ b/libmount/src/optlist.c
  68. @@ -875,7 +875,18 @@ int mnt_optlist_get_attrs(struct libmnt_optlist *ls, uint64_t *set, uint64_t *cl
  69. if (opt->ent->mask & MNT_INVERT) {
  70. DBG(OPTLIST, ul_debugobj(ls, " clr: %s", opt->ent->name));
  71. - *clr |= x;
  72. + /*
  73. + * All atime settings are mutually exclusive so *clr must
  74. + * have MOUNT_ATTR__ATIME set.
  75. + *
  76. + * See the function fs/namespace.c:build_mount_kattr()
  77. + * in the linux kernel source.
  78. + */
  79. + if (x == MOUNT_ATTR_RELATIME || x == MOUNT_ATTR_NOATIME ||
  80. + x == MOUNT_ATTR_STRICTATIME)
  81. + *clr |= MOUNT_ATTR__ATIME;
  82. + else
  83. + *clr |= x;
  84. } else {
  85. DBG(OPTLIST, ul_debugobj(ls, " set: %s", opt->ent->name));
  86. *set |= x;
  87. diff --git a/tests/expected/libmount/context-mount-flags b/tests/expected/libmount/context-mount-flags
  88. index 960641863..eb71323dd 100644
  89. --- a/tests/expected/libmount/context-mount-flags
  90. +++ b/tests/expected/libmount/context-mount-flags
  91. @@ -3,3 +3,6 @@ ro,nosuid,noexec
  92. successfully mounted
  93. rw,nosuid,noexec
  94. successfully umounted
  95. +successfully mounted
  96. +rw,relatime
  97. +successfully umounted
  98. diff --git a/tests/ts/libmount/context b/tests/ts/libmount/context
  99. index f5b47185e..a5d2e81a3 100755
  100. --- a/tests/ts/libmount/context
  101. +++ b/tests/ts/libmount/context
  102. @@ -116,8 +116,15 @@ $TS_CMD_FINDMNT --kernel --mountpoint $MOUNTPOINT -o VFS-OPTIONS -n >> $TS_OUTPU
  103. ts_run $TESTPROG --umount $MOUNTPOINT >> $TS_OUTPUT 2>> $TS_ERRLOG
  104. is_mounted $DEVICE && echo "$DEVICE still mounted" >> $TS_OUTPUT 2>> $TS_ERRLOG
  105. -ts_finalize_subtest
  106. +# Test that the atime option works after the migration to use the new kernel mount APIs.
  107. +ts_run $TESTPROG --mount -o atime $DEVICE $MOUNTPOINT >> $TS_OUTPUT 2>> $TS_ERRLOG
  108. +$TS_CMD_FINDMNT --kernel --mountpoint $MOUNTPOINT -o VFS-OPTIONS -n >> $TS_OUTPUT 2>> $TS_ERRLOG
  109. +is_mounted $DEVICE || echo "$DEVICE not mounted" >> $TS_OUTPUT 2>> $TS_ERRLOG
  110. +ts_run $TESTPROG --umount $MOUNTPOINT >> $TS_OUTPUT 2>> $TS_ERRLOG
  111. +is_mounted $DEVICE && echo "$DEVICE still mounted" >> $TS_OUTPUT 2>> $TS_ERRLOG
  112. +
  113. +ts_finalize_subtest
  114. ts_init_subtest "mount-loopdev"
  115. mkdir -p $MOUNTPOINT &> /dev/null
  116. --
  117. 2.40.1