[2.06 RELEASE PATCH v2 0/4] Add XFS bigtime and needsrepair support

[2.06 RELEASE PATCH v2 0/4] Add XFS bigtime and needsrepair support

[Javier Martinez Canillas]

Hello,

This series contains patches to support incompat feature flags that were
recently added to the XFS filesystem. This is needed because GRUB checks
if the super block .sb_features_incompat field contains incompat feature
flags that are not supported, and if that's the case it considers the SB
invalid and fails to read any data.

Since having a XFS filesystem with the bigtime or needsrepair flag would
lead to GRUB failing to access it, and due the fact that some distros are
going to turn them on by default (i.e: in the mkfs.xfs tool) I think that
these are 2.06 material.

This series is v2 of a collection of patches posted before separately to
the mailing list. It addresses the issues pointed out by Daniel on v1.

Best regards,
Javier

Changes in v2:
- Improve commit message for patch #2  (dkiper).
- Drop Reviewed-by tag from patch #2 (dkiper).
- A few code cleanups for patch #2 (dkiper).
- Improve the commit message of patch #3 (dkiper).
- Drop Reviewed-by tag from patch #3 (dkiper).
_ Drop unrelated changes in patch #3 (dkiper).
- Improve commit message of patch #4 (dkiper).
- Fix typo in debug message of patch #4 (dkiper).

Carlos Maiolino (2):
  fs: Use 64-bit type for filesystem timestamp
  fs/xfs: Add bigtime incompat feature support

Javier Martinez Canillas (2):
  types: Define PRI{x,d}GRUB_INT{32,64}_T format specifiers
  fs/xfs: Add needsrepair incompat feature support

 grub-core/fs/affs.c          |  2 +-
 grub-core/fs/ext2.c          |  2 +-
 grub-core/fs/fat.c           |  4 +-
 grub-core/fs/hfs.c           |  2 +-
 grub-core/fs/hfsplus.c       |  2 +-
 grub-core/fs/iso9660.c       |  6 +--
 grub-core/fs/nilfs2.c        |  2 +-
 grub-core/fs/squash4.c       |  2 +-
 grub-core/fs/ufs.c           |  2 +-
 grub-core/fs/xfs.c           | 78 ++++++++++++++++++++++++++++++------
 grub-core/fs/zfs/zfs.c       |  2 +-
 grub-core/lib/datetime.c     | 14 +++++--
 grub-core/net/bootp.c        |  2 +-
 grub-core/normal/misc.c      |  2 +-
 grub-core/tests/sleep_test.c |  4 +-
 include/grub/datetime.h      |  4 +-
 include/grub/fs.h            |  4 +-
 include/grub/types.h         |  6 +++
 18 files changed, 104 insertions(+), 36 deletions(-)

-- 
2.31.1

[2.06 RELEASE PATCH v2 1/4] types: Define PRI{x, d}GRUB_INT{32, 64}_T format specifiers

[Javier Martinez Canillas]

There are already constant macros defined for unsigned integers but no for
signed integers. Add macro constants for format specifiers of the latter.

Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

(no changes since v1)

 include/grub/types.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/grub/types.h b/include/grub/types.h
index b36b26a79d4..f460ef18c71 100644
--- a/include/grub/types.h
+++ b/include/grub/types.h
@@ -86,10 +86,16 @@
 typedef signed char		grub_int8_t;
 typedef short			grub_int16_t;
 typedef int			grub_int32_t;
+# define PRIxGRUB_INT32_T	"x"
+# define PRIuGRUB_INT32_T	"d"
 #if GRUB_CPU_SIZEOF_LONG == 8
 typedef long			grub_int64_t;
+# define PRIxGRUB_INT64_T	"lx"
+# define PRIdGRUB_INT64_T	"ld"
 #else
 typedef long long		grub_int64_t;
+# define PRIxGRUB_INT64_T	"llx"
+# define PRIdGRUB_INT64_T	"lld"
 #endif
 
 typedef unsigned char		grub_uint8_t;
-- 
2.31.1

[2.06 RELEASE PATCH v2 2/4] fs: Use 64-bit type for filesystem timestamp

[Javier Martinez Canillas]

From: Carlos Maiolino <cmaiolino@redhat.com>

Some filesystems nowadays uses 64-bit types for timestamps, so, update
grub_dirhook_info struct to use an grub_int64_t type to store mtime.
This also updates the grub_unixtime2datetime() function to receive a
64-bit timestamp argument and do 64-bit-safe divisions.

All the remaining conversion from 32-bit to 64-bit should be safe, as
32-bit to 64-bit attributions will be implicitly casted. The most
crictical part in the 32-bit to 64-bit conversion is on the function
grub_unixtime2datetime() where it needs to deal with the 64-bit type.
So, for that, the grub_divmod64() helper has been used.

These changes enables the GRUB to support dates beyond y2038.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v2:
- Improve commit message for patch #2  (dkiper).
- Drop Reviewed-by tag from patch #2 (dkiper).
- A few code cleanups for patch #2 (dkiper).

 grub-core/fs/affs.c          |  2 +-
 grub-core/fs/ext2.c          |  2 +-
 grub-core/fs/fat.c           |  4 ++--
 grub-core/fs/hfs.c           |  2 +-
 grub-core/fs/hfsplus.c       |  2 +-
 grub-core/fs/iso9660.c       |  6 +++---
 grub-core/fs/nilfs2.c        |  2 +-
 grub-core/fs/squash4.c       |  2 +-
 grub-core/fs/ufs.c           |  2 +-
 grub-core/fs/zfs/zfs.c       |  2 +-
 grub-core/lib/datetime.c     | 14 +++++++++++---
 grub-core/net/bootp.c        |  2 +-
 grub-core/normal/misc.c      |  2 +-
 grub-core/tests/sleep_test.c |  4 ++--
 include/grub/datetime.h      |  4 ++--
 include/grub/fs.h            |  4 ++--
 16 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/grub-core/fs/affs.c b/grub-core/fs/affs.c
index 230e26af0f8..cafcd0fba91 100644
--- a/grub-core/fs/affs.c
+++ b/grub-core/fs/affs.c
@@ -641,7 +641,7 @@ grub_affs_label (grub_device_t device, char **label)
 }
 
 static grub_err_t
-grub_affs_mtime (grub_device_t device, grub_int32_t *t)
+grub_affs_mtime (grub_device_t device, grub_int64_t *t)
 {
   struct grub_affs_data *data;
   grub_disk_t disk = device->disk;
diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
index 848bf939dba..e7dd78e6635 100644
--- a/grub-core/fs/ext2.c
+++ b/grub-core/fs/ext2.c
@@ -1055,7 +1055,7 @@ grub_ext2_uuid (grub_device_t device, char **uuid)
 
 /* Get mtime.  */
 static grub_err_t
-grub_ext2_mtime (grub_device_t device, grub_int32_t *tm)
+grub_ext2_mtime (grub_device_t device, grub_int64_t *tm)
 {
   struct grub_ext2_data *data;
   grub_disk_t disk = device->disk;
diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
index 7f775a17038..dd82e4ee35d 100644
--- a/grub-core/fs/fat.c
+++ b/grub-core/fs/fat.c
@@ -737,7 +737,7 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
  * https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification
  */
 static int
-grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int32_t *nix) {
+grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int64_t *nix) {
   struct grub_datetime datetime = {
     .year   = (field >> 25) + 1980,
     .month  = (field & 0x01E00000) >> 21,
@@ -891,7 +891,7 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
  * https://www.ecma-international.org/publications/files/ECMA-ST/Ecma-107.pdf
  */
 static int
-grub_fat_timestamp (grub_uint16_t time, grub_uint16_t date, grub_int32_t *nix) {
+grub_fat_timestamp (grub_uint16_t time, grub_uint16_t date, grub_int64_t *nix) {
   struct grub_datetime datetime = {
     .year   = (date >> 9) + 1980,
     .month  = (date & 0x01E0) >> 5,
diff --git a/grub-core/fs/hfs.c b/grub-core/fs/hfs.c
index 9a5b7bbe906..f419965d154 100644
--- a/grub-core/fs/hfs.c
+++ b/grub-core/fs/hfs.c
@@ -1374,7 +1374,7 @@ grub_hfs_label (grub_device_t device, char **label)
 }
 
 static grub_err_t
-grub_hfs_mtime (grub_device_t device, grub_int32_t *tm)
+grub_hfs_mtime (grub_device_t device, grub_int64_t *tm)
 {
   struct grub_hfs_data *data;
 
diff --git a/grub-core/fs/hfsplus.c b/grub-core/fs/hfsplus.c
index 2a69055c7ec..19c7b336798 100644
--- a/grub-core/fs/hfsplus.c
+++ b/grub-core/fs/hfsplus.c
@@ -1083,7 +1083,7 @@ grub_hfsplus_label (grub_device_t device, char **label)
 
 /* Get mtime.  */
 static grub_err_t
-grub_hfsplus_mtime (grub_device_t device, grub_int32_t *tm)
+grub_hfsplus_mtime (grub_device_t device, grub_int64_t *tm)
 {
   struct grub_hfsplus_data *data;
   grub_disk_t disk = device->disk;
diff --git a/grub-core/fs/iso9660.c b/grub-core/fs/iso9660.c
index 5ec4433b8f8..ac011950a64 100644
--- a/grub-core/fs/iso9660.c
+++ b/grub-core/fs/iso9660.c
@@ -178,7 +178,7 @@ static grub_dl_t my_mod;
 \f
 
 static grub_err_t
-iso9660_to_unixtime (const struct grub_iso9660_date *i, grub_int32_t *nix)
+iso9660_to_unixtime (const struct grub_iso9660_date *i, grub_int64_t *nix)
 {
   struct grub_datetime datetime;
   
@@ -206,7 +206,7 @@ iso9660_to_unixtime (const struct grub_iso9660_date *i, grub_int32_t *nix)
 }
 
 static int
-iso9660_to_unixtime2 (const struct grub_iso9660_date2 *i, grub_int32_t *nix)
+iso9660_to_unixtime2 (const struct grub_iso9660_date2 *i, grub_int64_t *nix)
 {
   struct grub_datetime datetime;
 
@@ -1107,7 +1107,7 @@ grub_iso9660_uuid (grub_device_t device, char **uuid)
 
 /* Get writing time of filesystem. */
 static grub_err_t 
-grub_iso9660_mtime (grub_device_t device, grub_int32_t *timebuf)
+grub_iso9660_mtime (grub_device_t device, grub_int64_t *timebuf)
 {
   struct grub_iso9660_data *data;
   grub_disk_t disk = device->disk;
diff --git a/grub-core/fs/nilfs2.c b/grub-core/fs/nilfs2.c
index 9b76982b3b0..3c248a910b4 100644
--- a/grub-core/fs/nilfs2.c
+++ b/grub-core/fs/nilfs2.c
@@ -1186,7 +1186,7 @@ grub_nilfs2_uuid (grub_device_t device, char **uuid)
 
 /* Get mtime.  */
 static grub_err_t
-grub_nilfs2_mtime (grub_device_t device, grub_int32_t * tm)
+grub_nilfs2_mtime (grub_device_t device, grub_int64_t * tm)
 {
   struct grub_nilfs2_data *data;
   grub_disk_t disk = device->disk;
diff --git a/grub-core/fs/squash4.c b/grub-core/fs/squash4.c
index a5f35c10e2f..6dd731e231e 100644
--- a/grub-core/fs/squash4.c
+++ b/grub-core/fs/squash4.c
@@ -1003,7 +1003,7 @@ grub_squash_close (grub_file_t file)
 }
 
 static grub_err_t
-grub_squash_mtime (grub_device_t dev, grub_int32_t *tm)
+grub_squash_mtime (grub_device_t dev, grub_int64_t *tm)
 {
   struct grub_squash_data *data = 0;
 
diff --git a/grub-core/fs/ufs.c b/grub-core/fs/ufs.c
index fca46baa19d..34a698b71b8 100644
--- a/grub-core/fs/ufs.c
+++ b/grub-core/fs/ufs.c
@@ -837,7 +837,7 @@ grub_ufs_uuid (grub_device_t device, char **uuid)
 
 /* Get mtime.  */
 static grub_err_t
-grub_ufs_mtime (grub_device_t device, grub_int32_t *tm)
+grub_ufs_mtime (grub_device_t device, grub_int64_t *tm)
 {
   struct grub_ufs_data *data = 0;
 
diff --git a/grub-core/fs/zfs/zfs.c b/grub-core/fs/zfs/zfs.c
index f9e755197c5..cf4d2ab189a 100644
--- a/grub-core/fs/zfs/zfs.c
+++ b/grub-core/fs/zfs/zfs.c
@@ -3771,7 +3771,7 @@ zfs_uuid (grub_device_t device, char **uuid)
 }
 
 static grub_err_t 
-zfs_mtime (grub_device_t device, grub_int32_t *mt)
+zfs_mtime (grub_device_t device, grub_int64_t *mt)
 {
   struct grub_zfs_data *data;
   grub_zfs_endian_t ub_endian = GRUB_ZFS_UNKNOWN_ENDIAN;
diff --git a/grub-core/lib/datetime.c b/grub-core/lib/datetime.c
index 95b8c9ff5e3..67415a57719 100644
--- a/grub-core/lib/datetime.c
+++ b/grub-core/lib/datetime.c
@@ -17,8 +17,10 @@
  *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <stddef.h>
 #include <grub/datetime.h>
 #include <grub/i18n.h>
+#include <grub/misc.h>
 
 static const char *const grub_weekday_names[] =
 {
@@ -60,7 +62,7 @@ grub_get_weekday_name (struct grub_datetime *datetime)
 
 
 void
-grub_unixtime2datetime (grub_int32_t nix, struct grub_datetime *datetime)
+grub_unixtime2datetime (grub_int64_t nix, struct grub_datetime *datetime)
 {
   int i;
   grub_uint8_t months[12] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
@@ -75,9 +77,15 @@ grub_unixtime2datetime (grub_int32_t nix, struct grub_datetime *datetime)
   unsigned secs_in_day;
   /* Transform C divisions and modulos to mathematical ones */
   if (nix < 0)
-    days_epoch = -(((unsigned) (SECPERDAY-nix-1)) / SECPERDAY);
+    /*
+     * the division here shouldn't be larger than INT_MAX,
+     * so, it's safe to store the result back in an int
+     */
+    days_epoch = -(grub_divmod64 (((grub_int64_t)(SECPERDAY) - nix - 1),
+                                  SECPERDAY, NULL));
   else
-    days_epoch = ((unsigned) nix) / SECPERDAY;
+    days_epoch = grub_divmod64 (nix, SECPERDAY, NULL);
+
   secs_in_day = nix - days_epoch * SECPERDAY;
   days = days_epoch + 69 * DAYSPERYEAR + 17;
 
diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
index e33be51f83b..6fb5627025d 100644
--- a/grub-core/net/bootp.c
+++ b/grub-core/net/bootp.c
@@ -468,7 +468,7 @@ send_dhcp_packet (struct grub_net_network_level_interface *iface)
   grub_err_t err;
   struct grub_net_bootp_packet *pack;
   struct grub_datetime date;
-  grub_int32_t t = 0;
+  grub_int64_t t = 0;
   struct grub_net_buff *nb;
   struct udphdr *udph;
   grub_net_network_level_address_t target;
diff --git a/grub-core/normal/misc.c b/grub-core/normal/misc.c
index 8bb6da31fb3..f7e9e3ac4a1 100644
--- a/grub-core/normal/misc.c
+++ b/grub-core/normal/misc.c
@@ -136,7 +136,7 @@ grub_normal_print_device_info (const char *name)
 	    }
 	  if (fs->fs_mtime)
 	    {
-	      grub_int32_t tm;
+	      grub_int64_t tm;
 	      struct grub_datetime datetime;
 	      (fs->fs_mtime) (dev, &tm);
 	      if (grub_errno == GRUB_ERR_NONE)
diff --git a/grub-core/tests/sleep_test.c b/grub-core/tests/sleep_test.c
index 3d11c717cb7..413577f4e9c 100644
--- a/grub-core/tests/sleep_test.c
+++ b/grub-core/tests/sleep_test.c
@@ -32,7 +32,7 @@ static void
 sleep_test (void)
 {
   struct grub_datetime st, en;
-  grub_int32_t stu = 0, enu = 0;
+  grub_int64_t stu = 0, enu = 0;
   int is_delayok;
   grub_test_assert (!grub_get_datetime (&st), "Couldn't retrieve start time");
   grub_millisleep (10000);
@@ -45,7 +45,7 @@ sleep_test (void)
   if (enu - stu >= 15 && enu - stu <= 17)
     is_delayok = 1;
 #endif
-  grub_test_assert (is_delayok, "Interval out of range: %d", enu-stu);
+  grub_test_assert (is_delayok, "Interval out of range: " PRIdGRUB_INT64_T, enu - stu);
 
 }
 
diff --git a/include/grub/datetime.h b/include/grub/datetime.h
index fef281404d7..23ae0791ced 100644
--- a/include/grub/datetime.h
+++ b/include/grub/datetime.h
@@ -48,11 +48,11 @@ grub_err_t grub_set_datetime (struct grub_datetime *datetime);
 int grub_get_weekday (struct grub_datetime *datetime);
 const char *grub_get_weekday_name (struct grub_datetime *datetime);
 
-void grub_unixtime2datetime (grub_int32_t nix,
+void grub_unixtime2datetime (grub_int64_t nix,
 			     struct grub_datetime *datetime);
 
 static inline int
-grub_datetime2unixtime (const struct grub_datetime *datetime, grub_int32_t *nix)
+grub_datetime2unixtime (const struct grub_datetime *datetime, grub_int64_t *nix)
 {
   grub_int32_t ret;
   int y4, ay;
diff --git a/include/grub/fs.h b/include/grub/fs.h
index 302e48d4b50..026bc3bb861 100644
--- a/include/grub/fs.h
+++ b/include/grub/fs.h
@@ -39,7 +39,7 @@ struct grub_dirhook_info
   unsigned mtimeset:1;
   unsigned case_insensitive:1;
   unsigned inodeset:1;
-  grub_int32_t mtime;
+  grub_int64_t mtime;
   grub_uint64_t inode;
 };
 
@@ -81,7 +81,7 @@ struct grub_fs
   grub_err_t (*fs_uuid) (grub_device_t device, char **uuid);
 
   /* Get writing time of filesystem. */
-  grub_err_t (*fs_mtime) (grub_device_t device, grub_int32_t *timebuf);
+  grub_err_t (*fs_mtime) (grub_device_t device, grub_int64_t *timebuf);
 
 #ifdef GRUB_UTIL
   /* Determine sectors available for embedding.  */
-- 
2.31.1

[2.06 RELEASE PATCH v2 3/4] fs/xfs: Add bigtime incompat feature support

[Javier Martinez Canillas]

From: Carlos Maiolino <cmaiolino@redhat.com>

The XFS filesystem supports a bigtime feature to overcome y2038 problem.
This patch makes the GRUB able to support the XFS filesystems with this
feature enabled.

The XFS counter for the bigtime enabled timestamps starts on 0, which
translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy
timestamps. The conversion to unix timestamps is made before passing the
value to other GRUB functions.

For this to work properly, GRUB requires to access flags2 field in the
XFS ondisk inode. So, the grub_xfs_inode structure has been updated to
the full ondisk inode size.

This patch is enough to make the GRUB work properly with files that have
timestamps with values up to GRUB_INT32_MAX (Jan 19 03:14:07 UTC 2038).

Any file with a timestamps bigger than this will overflow the counter,
causing GRUB to show wrong timestamps. This is not really different than
the current situation, since the timestamps in GRUB are 32-bit values.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v2:
- Improve the commit message of patch #3 (dkiper).
- Drop Reviewed-by tag from patch #3 (dkiper).
_ Drop unrelated changes in patch #3 (dkiper).

 grub-core/fs/xfs.c | 63 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 51 insertions(+), 12 deletions(-)

diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
index 43023e03fb3..bcf281cf80d 100644
--- a/grub-core/fs/xfs.c
+++ b/grub-core/fs/xfs.c
@@ -75,10 +75,15 @@ GRUB_MOD_LICENSE ("GPLv3+");
 	 XFS_SB_VERSION2_PROJID32BIT | \
 	 XFS_SB_VERSION2_FTYPE)
 
+/* Inode flags2 flags */
+#define XFS_DIFLAG2_BIGTIME_BIT	3
+#define XFS_DIFLAG2_BIGTIME	(1 << XFS_DIFLAG2_BIGTIME_BIT)
+
 /* incompat feature flags */
 #define XFS_SB_FEAT_INCOMPAT_FTYPE      (1 << 0)        /* filetype in dirent */
 #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
+#define XFS_SB_FEAT_INCOMPAT_BIGTIME    (1 << 3)        /* large timestamps */
 
 /*
  * Directory entries with ftype are explicitly handled by GRUB code.
@@ -92,7 +97,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
 #define XFS_SB_FEAT_INCOMPAT_SUPPORTED \
 	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
 	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
-	 XFS_SB_FEAT_INCOMPAT_META_UUID)
+	 XFS_SB_FEAT_INCOMPAT_META_UUID | \
+	 XFS_SB_FEAT_INCOMPAT_BIGTIME)
 
 struct grub_xfs_sblock
 {
@@ -177,7 +183,7 @@ struct grub_xfs_btree_root
   grub_uint64_t keys[1];
 } GRUB_PACKED;
 
-struct grub_xfs_time
+struct grub_xfs_time_legacy
 {
   grub_uint32_t sec;
   grub_uint32_t nanosec;
@@ -190,20 +196,23 @@ struct grub_xfs_inode
   grub_uint8_t version;
   grub_uint8_t format;
   grub_uint8_t unused2[26];
-  struct grub_xfs_time atime;
-  struct grub_xfs_time mtime;
-  struct grub_xfs_time ctime;
+  grub_uint64_t atime;
+  grub_uint64_t mtime;
+  grub_uint64_t ctime;
   grub_uint64_t size;
   grub_uint64_t nblocks;
   grub_uint32_t extsize;
   grub_uint32_t nextents;
   grub_uint16_t unused3;
   grub_uint8_t fork_offset;
-  grub_uint8_t unused4[17];
+  grub_uint8_t unused4[37];
+  grub_uint64_t flags2;
+  grub_uint8_t unused5[48];
 } GRUB_PACKED;
 
-#define XFS_V2_INODE_SIZE sizeof(struct grub_xfs_inode)
-#define XFS_V3_INODE_SIZE (XFS_V2_INODE_SIZE + 76)
+#define XFS_V3_INODE_SIZE sizeof(struct grub_xfs_inode)
+/* Size of struct grub_xfs_inode until fork_offset (included)*/
+#define XFS_V2_INODE_SIZE (XFS_V3_INODE_SIZE - 92)
 
 struct grub_xfs_dirblock_tail
 {
@@ -233,8 +242,6 @@ struct grub_xfs_data
 
 static grub_dl_t my_mod;
 
-\f
-
 static int grub_xfs_sb_hascrc(struct grub_xfs_data *data)
 {
   return (data->sblock.version & grub_cpu_to_be16_compile_time(XFS_SB_VERSION_NUMBITS)) ==
@@ -950,7 +957,6 @@ grub_xfs_mount (grub_disk_t disk)
   return 0;
 }
 
-\f
 /* Context for grub_xfs_dir.  */
 struct grub_xfs_dir_ctx
 {
@@ -958,6 +964,39 @@ struct grub_xfs_dir_ctx
   void *hook_data;
 };
 
+/* Bigtime inodes helpers */
+
+#define NSEC_PER_SEC 1000000000L
+#define XFS_BIGTIME_EPOCH_OFFSET (-(grub_int64_t)GRUB_INT32_MIN)
+
+static int grub_xfs_inode_has_bigtime(const struct grub_xfs_inode *inode)
+{
+  return inode->version >= 3 &&
+	(inode->flags2 & grub_cpu_to_be64_compile_time(XFS_DIFLAG2_BIGTIME));
+}
+
+static grub_int64_t
+grub_xfs_bigtime_to_unix(grub_uint64_t time)
+{
+  grub_uint64_t rem;
+  grub_int64_t nsec = NSEC_PER_SEC;
+  grub_int64_t seconds = grub_divmod64((grub_int64_t)time, nsec, &rem);
+
+  return seconds - XFS_BIGTIME_EPOCH_OFFSET;
+}
+
+static grub_int64_t
+grub_xfs_get_inode_time(struct grub_xfs_inode *inode)
+{
+  struct grub_xfs_time_legacy *lts;
+
+  if (grub_xfs_inode_has_bigtime(inode))
+    return grub_xfs_bigtime_to_unix(grub_be_to_cpu64(inode->mtime));
+
+  lts = (struct grub_xfs_time_legacy *)&inode->mtime;
+  return grub_be_to_cpu32(lts->sec);
+}
+
 /* Helper for grub_xfs_dir.  */
 static int
 grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype,
@@ -970,7 +1009,7 @@ grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype,
   if (node->inode_read)
     {
       info.mtimeset = 1;
-      info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec);
+      info.mtime = grub_xfs_get_inode_time(&node->inode);
     }
   info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
   grub_free (node);
-- 
2.31.1

[2.06 RELEASE PATCH v2 4/4] fs/xfs: Add needsrepair incompat feature support

[Javier Martinez Canillas]

The XFS now has an incompat feature flag to indicate that the filesystem
needs to be repaired. The Linux kernel refuses to mount a filesystem that
has it set and only the xfs_repair tool is able to clear that flag.

The GRUB doesn't have the concept of mounting filesystems and just attempt
to read the files. But it does some sanity checking before attempting to
read from a filesystem.

Among the things which are tested, is if the super block only has set the
incompatible features flags that are supported by GRUB. If it contains any
flags that are not listed as supported, reading the XFS filesystem fails.

Since the GRUB doesn't attempt to detect if the filesystem is inconsistent
nor replays the journal, just ignore if a filesystem needs to be repaired.

For this reason, make it a best effort but print a debug message when the
XFS filesystem is accessed if should be repaired. That way, if the reading
or later booting fail, the user can figure out that may be related to this.

Suggested-by: Eric Sandeen <esandeen@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

Changes in v2:
- Improve commit message of patch #4 (dkiper).
- Fix typo in debug message of patch #4 (dkiper).

 grub-core/fs/xfs.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
index bcf281cf80d..6056bf44e32 100644
--- a/grub-core/fs/xfs.c
+++ b/grub-core/fs/xfs.c
@@ -84,6 +84,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
 #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
 #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
 #define XFS_SB_FEAT_INCOMPAT_BIGTIME    (1 << 3)        /* large timestamps */
+#define XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR (1 << 4)       /* needs xfs_repair */
 
 /*
  * Directory entries with ftype are explicitly handled by GRUB code.
@@ -98,7 +99,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
 	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
 	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
 	 XFS_SB_FEAT_INCOMPAT_META_UUID | \
-	 XFS_SB_FEAT_INCOMPAT_BIGTIME)
+	 XFS_SB_FEAT_INCOMPAT_BIGTIME | \
+	 XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR)
 
 struct grub_xfs_sblock
 {
@@ -307,6 +309,16 @@ static int grub_xfs_sb_valid(struct grub_xfs_data *data)
   return 0;
 }
 
+static int
+grub_xfs_sb_needs_repair (struct grub_xfs_data *data)
+{
+  return ((data->sblock.version &
+           grub_cpu_to_be16_compile_time (XFS_SB_VERSION_NUMBITS)) ==
+          grub_cpu_to_be16_compile_time (XFS_SB_VERSION_5) &&
+          (data->sblock.sb_features_incompat &
+           grub_cpu_to_be32_compile_time (XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR)));
+}
+
 /* Filetype information as used in inodes.  */
 #define FILETYPE_INO_MASK	0170000
 #define FILETYPE_INO_REG	0100000
@@ -922,6 +934,9 @@ grub_xfs_mount (grub_disk_t disk)
   if (!grub_xfs_sb_valid(data))
     goto fail;
 
+  if (grub_xfs_sb_needs_repair (data))
+    grub_dprintf ("xfs", "XFS filesystem needs repair, can lead to failures\n");
+
   if (grub_add (grub_xfs_inode_size (data),
       sizeof (struct grub_xfs_data) - sizeof (struct grub_xfs_inode) + 1, &sz))
     goto fail;
-- 
2.31.1

Re: [2.06 RELEASE PATCH v2 1/4] types: Define PRI{x,d}GRUB_INT{32,64}_T format specifiers

[Daniel Kiper]

On Wed, May 12, 2021 at 05:46:13PM +0200, Javier Martinez Canillas wrote:
> There are already constant macros defined for unsigned integers but no for
> signed integers. Add macro constants for format specifiers of the latter.
>
> Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> except...

> ---
>
> (no changes since v1)
>
>  include/grub/types.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/grub/types.h b/include/grub/types.h
> index b36b26a79d4..f460ef18c71 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -86,10 +86,16 @@
>  typedef signed char		grub_int8_t;
>  typedef short			grub_int16_t;
>  typedef int			grub_int32_t;
> +# define PRIxGRUB_INT32_T	"x"
> +# define PRIuGRUB_INT32_T	"d"
               ^ <---- s/u/d/

Though I can fix it before commiting...

>  #if GRUB_CPU_SIZEOF_LONG == 8
>  typedef long			grub_int64_t;
> +# define PRIxGRUB_INT64_T	"lx"
> +# define PRIdGRUB_INT64_T	"ld"
>  #else
>  typedef long long		grub_int64_t;
> +# define PRIxGRUB_INT64_T	"llx"
> +# define PRIdGRUB_INT64_T	"lld"
>  #endif

Daniel

Re: [2.06 RELEASE PATCH v2 3/4] fs/xfs: Add bigtime incompat feature support

[Daniel Kiper]

On Wed, May 12, 2021 at 05:46:15PM +0200, Javier Martinez Canillas wrote:
> From: Carlos Maiolino <cmaiolino@redhat.com>
>
> The XFS filesystem supports a bigtime feature to overcome y2038 problem.
> This patch makes the GRUB able to support the XFS filesystems with this
> feature enabled.
>
> The XFS counter for the bigtime enabled timestamps starts on 0, which
> translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy
> timestamps. The conversion to unix timestamps is made before passing the
> value to other GRUB functions.
>
> For this to work properly, GRUB requires to access flags2 field in the
> XFS ondisk inode. So, the grub_xfs_inode structure has been updated to
> the full ondisk inode size.

s/the full ondisk inode size/cover full ondisk inode/?

> This patch is enough to make the GRUB work properly with files that have
> timestamps with values up to GRUB_INT32_MAX (Jan 19 03:14:07 UTC 2038).

This paragraph contradicts a bit what was said in the first paragraph...

> Any file with a timestamps bigger than this will overflow the counter,
> causing GRUB to show wrong timestamps. This is not really different than
> the current situation, since the timestamps in GRUB are 32-bit values.

Err... I think this is incorrect right now because the patch before this
one fixed the issue. So, this paragraph should be dropped. There is only
one question: are the changes here sufficient to have full XFS bigtime
support in the GRUB? I think they are...

> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
> Changes in v2:
> - Improve the commit message of patch #3 (dkiper).
> - Drop Reviewed-by tag from patch #3 (dkiper).
> _ Drop unrelated changes in patch #3 (dkiper).
>
>  grub-core/fs/xfs.c | 63 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index 43023e03fb3..bcf281cf80d 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -75,10 +75,15 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  	 XFS_SB_VERSION2_PROJID32BIT | \
>  	 XFS_SB_VERSION2_FTYPE)
>
> +/* Inode flags2 flags */
> +#define XFS_DIFLAG2_BIGTIME_BIT	3
> +#define XFS_DIFLAG2_BIGTIME	(1 << XFS_DIFLAG2_BIGTIME_BIT)
> +
>  /* incompat feature flags */
>  #define XFS_SB_FEAT_INCOMPAT_FTYPE      (1 << 0)        /* filetype in dirent */
>  #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
>  #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
> +#define XFS_SB_FEAT_INCOMPAT_BIGTIME    (1 << 3)        /* large timestamps */
>
>  /*
>   * Directory entries with ftype are explicitly handled by GRUB code.
> @@ -92,7 +97,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define XFS_SB_FEAT_INCOMPAT_SUPPORTED \
>  	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
>  	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
> -	 XFS_SB_FEAT_INCOMPAT_META_UUID)
> +	 XFS_SB_FEAT_INCOMPAT_META_UUID | \
> +	 XFS_SB_FEAT_INCOMPAT_BIGTIME)
>
>  struct grub_xfs_sblock
>  {
> @@ -177,7 +183,7 @@ struct grub_xfs_btree_root
>    grub_uint64_t keys[1];
>  } GRUB_PACKED;
>
> -struct grub_xfs_time
> +struct grub_xfs_time_legacy
>  {
>    grub_uint32_t sec;
>    grub_uint32_t nanosec;
> @@ -190,20 +196,23 @@ struct grub_xfs_inode
>    grub_uint8_t version;
>    grub_uint8_t format;
>    grub_uint8_t unused2[26];
> -  struct grub_xfs_time atime;
> -  struct grub_xfs_time mtime;
> -  struct grub_xfs_time ctime;
> +  grub_uint64_t atime;
> +  grub_uint64_t mtime;
> +  grub_uint64_t ctime;
>    grub_uint64_t size;
>    grub_uint64_t nblocks;
>    grub_uint32_t extsize;
>    grub_uint32_t nextents;
>    grub_uint16_t unused3;
>    grub_uint8_t fork_offset;
> -  grub_uint8_t unused4[17];
> +  grub_uint8_t unused4[37];
> +  grub_uint64_t flags2;
> +  grub_uint8_t unused5[48];
>  } GRUB_PACKED;
>
> -#define XFS_V2_INODE_SIZE sizeof(struct grub_xfs_inode)
> -#define XFS_V3_INODE_SIZE (XFS_V2_INODE_SIZE + 76)
> +#define XFS_V3_INODE_SIZE sizeof(struct grub_xfs_inode)
> +/* Size of struct grub_xfs_inode until fork_offset (included)*/
> +#define XFS_V2_INODE_SIZE (XFS_V3_INODE_SIZE - 92)
>
>  struct grub_xfs_dirblock_tail
>  {
> @@ -233,8 +242,6 @@ struct grub_xfs_data
>
>  static grub_dl_t my_mod;
>
> -\f
> -

Could you drop this change?

>  static int grub_xfs_sb_hascrc(struct grub_xfs_data *data)
>  {
>    return (data->sblock.version & grub_cpu_to_be16_compile_time(XFS_SB_VERSION_NUMBITS)) ==
> @@ -950,7 +957,6 @@ grub_xfs_mount (grub_disk_t disk)
>    return 0;
>  }
>
> -\f

Ditto...

>  /* Context for grub_xfs_dir.  */
>  struct grub_xfs_dir_ctx
>  {
> @@ -958,6 +964,39 @@ struct grub_xfs_dir_ctx
>    void *hook_data;
>  };
>
> +/* Bigtime inodes helpers */
> +
> +#define NSEC_PER_SEC 1000000000L
> +#define XFS_BIGTIME_EPOCH_OFFSET (-(grub_int64_t)GRUB_INT32_MIN)

I think starting from here you forgot to take into account my comments...

> +static int grub_xfs_inode_has_bigtime(const struct grub_xfs_inode *inode)
> +{
> +  return inode->version >= 3 &&
> +	(inode->flags2 & grub_cpu_to_be64_compile_time(XFS_DIFLAG2_BIGTIME));
> +}
> +
> +static grub_int64_t
> +grub_xfs_bigtime_to_unix(grub_uint64_t time)
> +{
> +  grub_uint64_t rem;
> +  grub_int64_t nsec = NSEC_PER_SEC;
> +  grub_int64_t seconds = grub_divmod64((grub_int64_t)time, nsec, &rem);
> +
> +  return seconds - XFS_BIGTIME_EPOCH_OFFSET;
> +}
> +
> +static grub_int64_t
> +grub_xfs_get_inode_time(struct grub_xfs_inode *inode)
> +{
> +  struct grub_xfs_time_legacy *lts;
> +
> +  if (grub_xfs_inode_has_bigtime(inode))
> +    return grub_xfs_bigtime_to_unix(grub_be_to_cpu64(inode->mtime));
> +
> +  lts = (struct grub_xfs_time_legacy *)&inode->mtime;
> +  return grub_be_to_cpu32(lts->sec);
> +}
> +
>  /* Helper for grub_xfs_dir.  */
>  static int
>  grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype,
> @@ -970,7 +1009,7 @@ grub_xfs_dir_iter (const char *filename, enum grub_fshelp_filetype filetype,
>    if (node->inode_read)
>      {
>        info.mtimeset = 1;
> -      info.mtime = grub_be_to_cpu32 (node->inode.mtime.sec);
> +      info.mtime = grub_xfs_get_inode_time(&node->inode);
>      }
>    info.dir = ((filetype & GRUB_FSHELP_TYPE_MASK) == GRUB_FSHELP_DIR);
>    grub_free (node);

Daniel

Re: [2.06 RELEASE PATCH v2 4/4] fs/xfs: Add needsrepair incompat feature support

[Daniel Kiper]

On Wed, May 12, 2021 at 05:46:16PM +0200, Javier Martinez Canillas wrote:
> The XFS now has an incompat feature flag to indicate that the filesystem
> needs to be repaired. The Linux kernel refuses to mount a filesystem that
> has it set and only the xfs_repair tool is able to clear that flag.
>
> The GRUB doesn't have the concept of mounting filesystems and just attempt

s/attempt/attempts/

> to read the files. But it does some sanity checking before attempting to
> read from a filesystem.
>
> Among the things which are tested, is if the super block only has set the
> incompatible features flags that are supported by GRUB. If it contains any
> flags that are not listed as supported, reading the XFS filesystem fails.
>
> Since the GRUB doesn't attempt to detect if the filesystem is inconsistent
> nor replays the journal, just ignore if a filesystem needs to be repaired.

This sentence does not parse well...

> For this reason, make it a best effort but print a debug message when the

s/but/and/

> XFS filesystem is accessed if should be repaired. That way, if the reading

s/is accessed if/accessed/

> or later booting fail, the user can figure out that may be related to this.

s/if the reading or later booting fail/if later reading or booting fails/

> Suggested-by: Eric Sandeen <esandeen@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
> Changes in v2:
> - Improve commit message of patch #4 (dkiper).
> - Fix typo in debug message of patch #4 (dkiper).
>
>  grub-core/fs/xfs.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c
> index bcf281cf80d..6056bf44e32 100644
> --- a/grub-core/fs/xfs.c
> +++ b/grub-core/fs/xfs.c
> @@ -84,6 +84,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define XFS_SB_FEAT_INCOMPAT_SPINODES   (1 << 1)        /* sparse inode chunks */
>  #define XFS_SB_FEAT_INCOMPAT_META_UUID  (1 << 2)        /* metadata UUID */
>  #define XFS_SB_FEAT_INCOMPAT_BIGTIME    (1 << 3)        /* large timestamps */
> +#define XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR (1 << 4)       /* needs xfs_repair */
>
>  /*
>   * Directory entries with ftype are explicitly handled by GRUB code.
> @@ -98,7 +99,8 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  	(XFS_SB_FEAT_INCOMPAT_FTYPE | \
>  	 XFS_SB_FEAT_INCOMPAT_SPINODES | \
>  	 XFS_SB_FEAT_INCOMPAT_META_UUID | \
> -	 XFS_SB_FEAT_INCOMPAT_BIGTIME)
> +	 XFS_SB_FEAT_INCOMPAT_BIGTIME | \
> +	 XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR)
>
>  struct grub_xfs_sblock
>  {
> @@ -307,6 +309,16 @@ static int grub_xfs_sb_valid(struct grub_xfs_data *data)
>    return 0;
>  }
>
> +static int
> +grub_xfs_sb_needs_repair (struct grub_xfs_data *data)
> +{
> +  return ((data->sblock.version &
> +           grub_cpu_to_be16_compile_time (XFS_SB_VERSION_NUMBITS)) ==
> +          grub_cpu_to_be16_compile_time (XFS_SB_VERSION_5) &&
> +          (data->sblock.sb_features_incompat &
> +           grub_cpu_to_be32_compile_time (XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR)));
> +}
> +
>  /* Filetype information as used in inodes.  */
>  #define FILETYPE_INO_MASK	0170000
>  #define FILETYPE_INO_REG	0100000
> @@ -922,6 +934,9 @@ grub_xfs_mount (grub_disk_t disk)
>    if (!grub_xfs_sb_valid(data))
>      goto fail;
>
> +  if (grub_xfs_sb_needs_repair (data))
> +    grub_dprintf ("xfs", "XFS filesystem needs repair, can lead to failures\n");

s/can lead to failures/boot may fail/

Daniel

Re: [2.06 RELEASE PATCH v2 3/4] fs/xfs: Add bigtime incompat feature support

[Javier Martinez Canillas]

Hello Daniel,

On 5/17/21 4:09 PM, Daniel Kiper wrote:
> On Wed, May 12, 2021 at 05:46:15PM +0200, Javier Martinez Canillas wrote:
>> From: Carlos Maiolino <cmaiolino@redhat.com>
>>
>> The XFS filesystem supports a bigtime feature to overcome y2038 problem.
>> This patch makes the GRUB able to support the XFS filesystems with this
>> feature enabled.
>>
>> The XFS counter for the bigtime enabled timestamps starts on 0, which
>> translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy
>> timestamps. The conversion to unix timestamps is made before passing the
>> value to other GRUB functions.
>>
>> For this to work properly, GRUB requires to access flags2 field in the
>> XFS ondisk inode. So, the grub_xfs_inode structure has been updated to
>> the full ondisk inode size.
> 
> s/the full ondisk inode size/cover full ondisk inode/?
>

Ok.
 
>> This patch is enough to make the GRUB work properly with files that have
>> timestamps with values up to GRUB_INT32_MAX (Jan 19 03:14:07 UTC 2038).
> 
> This paragraph contradicts a bit what was said in the first paragraph...
>
>> Any file with a timestamps bigger than this will overflow the counter,
>> causing GRUB to show wrong timestamps. This is not really different than
>> the current situation, since the timestamps in GRUB are 32-bit values.
> 
> Err... I think this is incorrect right now because the patch before this
> one fixed the issue. So, this paragraph should be dropped. There is only
> one question: are the changes here sufficient to have full XFS bigtime
> support in the GRUB? I think they are...
>

Yes, sorry about that. Originally Carlos' series had this patch before 1/4
and I re-arranged them in the series because made more sense to first add
the 64-bit support in GRUB core and then the XFS support. I'll drop this.

[snip]

>>
>>  static grub_dl_t my_mod;
>>
>> -\f
>> -
>

Ok.
 
> Could you drop this change?
> 
>>  static int grub_xfs_sb_hascrc(struct grub_xfs_data *data)
>>  {
>>    return (data->sblock.version & grub_cpu_to_be16_compile_time(XFS_SB_VERSION_NUMBITS)) ==
>> @@ -950,7 +957,6 @@ grub_xfs_mount (grub_disk_t disk)
>>    return 0;
>>  }
>>
>> -\f
> 
> Ditto...
>

Ok.
 
>>  /* Context for grub_xfs_dir.  */
>>  struct grub_xfs_dir_ctx
>>  {
>> @@ -958,6 +964,39 @@ struct grub_xfs_dir_ctx
>>    void *hook_data;
>>  };
>>
>> +/* Bigtime inodes helpers */
>> +
>> +#define NSEC_PER_SEC 1000000000L
>> +#define XFS_BIGTIME_EPOCH_OFFSET (-(grub_int64_t)GRUB_INT32_MIN)
> 
> I think starting from here you forgot to take into account my comments...
>

Oh, I did indeed miss these comments... sorry, I'll fix them in v3.

Best regards, -- 
Javier Martinez Canillas
Software Engineer
New Platform Technologies Enablement team
RHEL Engineering