[PATCH v2 2/2] isofs: use unsigned char types consistently

From: Arnd Bergmann
Date: Thu Oct 19 2017 - 10:48:46 EST

Next message: David Howells: "[PATCH 00/27] security, efi: Add kernel lockdown"
Previous message: Arnd Bergmann: "[PATCH v2 1/2] isofs: fix timestamps beyond 2027"
In reply to: Arnd Bergmann: "[PATCH v2 1/2] isofs: fix timestamps beyond 2027"
Next in thread: Jan Kara: "Re: [PATCH v2 1/2] isofs: fix timestamps beyond 2027"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Based on the discussion about the signed character field for the year,
I went through all fields in the iso9660 and rockridge standards to see
whether they should used signed or unsigned characters. Only a single
8-bit value is defined as signed per 'section 7.1.2': the timezone
offset in a timestamp, this has always been handled correctly through
explicit sign-extension.

All others are either '7.1.1 8-bit unsigned numerical values' or
composite fields. I also read the linux source code and came to the
same conclusion, also I could not find any other part of the
implementation that actually behaves differently for signed or
unsigned values.

Since it is still ambigous to use plain 'char' in interface definitions,
I'm changing all fields representing numbers and reserved bytes to
the unambiguous '__u8'. Fields that hold actual strings are left as
'char' arrays. I built the code with '-Wpointer-sign -Wsign-compare'
to see if anything got left out, but couldn't find anything wrong
with the remaining warnings.

This patch should not change runtime behavior and does not need to
be backported.

Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
---
fs/isofs/isofs.h | 20 +++---
fs/isofs/rock.h | 62 ++++++++---------
include/uapi/linux/iso_fs.h | 162 ++++++++++++++++++++++----------------------
3 files changed, 122 insertions(+), 122 deletions(-)

diff --git a/fs/isofs/isofs.h b/fs/isofs/isofs.h
index bd4047585431..c882f207dd5c 100644
--- a/fs/isofs/isofs.h
+++ b/fs/isofs/isofs.h
@@ -72,36 +72,36 @@ static inline struct iso_inode_info *ISOFS_I(struct inode *inode)
return container_of(inode, struct iso_inode_info, vfs_inode);
}

-static inline int isonum_711(char *p)
+static inline int isonum_711(u8 *p)
{
- return *(u8 *)p;
+ return *p;
}
-static inline int isonum_712(char *p)
+static inline int isonum_712(s8 *p)
{
- return *(s8 *)p;
+ return *p;
}
-static inline unsigned int isonum_721(char *p)
+static inline unsigned int isonum_721(u8 *p)
{
return get_unaligned_le16(p);
}
-static inline unsigned int isonum_722(char *p)
+static inline unsigned int isonum_722(u8 *p)
{
return get_unaligned_be16(p);
}
-static inline unsigned int isonum_723(char *p)
+static inline unsigned int isonum_723(u8 *p)
{
/* Ignore bigendian datum due to broken mastering programs */
return get_unaligned_le16(p);
}
-static inline unsigned int isonum_731(char *p)
+static inline unsigned int isonum_731(u8 *p)
{
return get_unaligned_le32(p);
}
-static inline unsigned int isonum_732(char *p)
+static inline unsigned int isonum_732(u8 *p)
{
return get_unaligned_be32(p);
}
-static inline unsigned int isonum_733(char *p)
+static inline unsigned int isonum_733(u8 *p)
{
/* Ignore bigendian datum due to broken mastering programs */
return get_unaligned_le32(p);
diff --git a/fs/isofs/rock.h b/fs/isofs/rock.h
index f835976ce033..8780d67c6ca5 100644
--- a/fs/isofs/rock.h
+++ b/fs/isofs/rock.h
@@ -6,62 +6,62 @@
*/

struct SU_SP_s {
- unsigned char magic[2];
- unsigned char skip;
+ __u8 magic[2];
+ __u8 skip;
} __attribute__ ((packed));

struct SU_CE_s {
- char extent[8];
- char offset[8];
- char size[8];
+ __u8 extent[8];
+ __u8 offset[8];
+ __u8 size[8];
};

struct SU_ER_s {
- unsigned char len_id;
- unsigned char len_des;
- unsigned char len_src;
- unsigned char ext_ver;
- char data[0];
+ __u8 len_id;
+ __u8 len_des;
+ __u8 len_src;
+ __u8 ext_ver;
+ __u8 data[0];
} __attribute__ ((packed));

struct RR_RR_s {
- char flags[1];
+ __u8 flags[1];
} __attribute__ ((packed));

struct RR_PX_s {
- char mode[8];
- char n_links[8];
- char uid[8];
- char gid[8];
+ __u8 mode[8];
+ __u8 n_links[8];
+ __u8 uid[8];
+ __u8 gid[8];
};

struct RR_PN_s {
- char dev_high[8];
- char dev_low[8];
+ __u8 dev_high[8];
+ __u8 dev_low[8];
};

struct SL_component {
- unsigned char flags;
- unsigned char len;
- char text[0];
+ __u8 flags;
+ __u8 len;
+ __u8 text[0];
} __attribute__ ((packed));

struct RR_SL_s {
- unsigned char flags;
+ __u8 flags;
struct SL_component link;
} __attribute__ ((packed));

struct RR_NM_s {
- unsigned char flags;
+ __u8 flags;
char name[0];
} __attribute__ ((packed));

struct RR_CL_s {
- char location[8];
+ __u8 location[8];
};

struct RR_PL_s {
- char location[8];
+ __u8 location[8];
};

struct stamp {
@@ -69,15 +69,15 @@ struct stamp {
} __attribute__ ((packed));

struct RR_TF_s {
- char flags;
+ __u8 flags;
struct stamp times[0]; /* Variable number of these beasts */
} __attribute__ ((packed));

/* Linux-specific extension for transparent decompression */
struct RR_ZF_s {
- char algorithm[2];
- char parms[2];
- char real_size[8];
+ __u8 algorithm[2];
+ __u8 parms[2];
+ __u8 real_size[8];
};

/*
@@ -93,9 +93,9 @@ struct RR_ZF_s {
#define TF_LONG_FORM 128

struct rock_ridge {
- char signature[2];
- unsigned char len;
- unsigned char version;
+ __u8 signature[2];
+ __u8 len;
+ __u8 version;
union {
struct SU_SP_s SP;
struct SU_CE_s CE;
diff --git a/include/uapi/linux/iso_fs.h b/include/uapi/linux/iso_fs.h
index 4688ac4284e2..07c4c6405b3c 100644
--- a/include/uapi/linux/iso_fs.h
+++ b/include/uapi/linux/iso_fs.h
@@ -12,10 +12,10 @@
#define ISODCL(from, to) (to - from + 1)

struct iso_volume_descriptor {
- char type[ISODCL(1,1)]; /* 711 */
+ __u8 type[ISODCL(1,1)]; /* 711 */
char id[ISODCL(2,6)];
- char version[ISODCL(7,7)];
- char data[ISODCL(8,2048)];
+ __u8 version[ISODCL(7,7)];
+ __u8 data[ISODCL(8,2048)];
};

/* volume descriptor types */
@@ -26,24 +26,24 @@ struct iso_volume_descriptor {
#define ISO_STANDARD_ID "CD001"

struct iso_primary_descriptor {
- char type [ISODCL ( 1, 1)]; /* 711 */
+ __u8 type [ISODCL ( 1, 1)]; /* 711 */
char id [ISODCL ( 2, 6)];
- char version [ISODCL ( 7, 7)]; /* 711 */
- char unused1 [ISODCL ( 8, 8)];
+ __u8 version [ISODCL ( 7, 7)]; /* 711 */
+ __u8 unused1 [ISODCL ( 8, 8)];
char system_id [ISODCL ( 9, 40)]; /* achars */
char volume_id [ISODCL ( 41, 72)]; /* dchars */
- char unused2 [ISODCL ( 73, 80)];
- char volume_space_size [ISODCL ( 81, 88)]; /* 733 */
- char unused3 [ISODCL ( 89, 120)];
- char volume_set_size [ISODCL (121, 124)]; /* 723 */
- char volume_sequence_number [ISODCL (125, 128)]; /* 723 */
- char logical_block_size [ISODCL (129, 132)]; /* 723 */
- char path_table_size [ISODCL (133, 140)]; /* 733 */
- char type_l_path_table [ISODCL (141, 144)]; /* 731 */
- char opt_type_l_path_table [ISODCL (145, 148)]; /* 731 */
- char type_m_path_table [ISODCL (149, 152)]; /* 732 */
- char opt_type_m_path_table [ISODCL (153, 156)]; /* 732 */
- char root_directory_record [ISODCL (157, 190)]; /* 9.1 */
+ __u8 unused2 [ISODCL ( 73, 80)];
+ __u8 volume_space_size [ISODCL ( 81, 88)]; /* 733 */
+ __u8 unused3 [ISODCL ( 89, 120)];
+ __u8 volume_set_size [ISODCL (121, 124)]; /* 723 */
+ __u8 volume_sequence_number [ISODCL (125, 128)]; /* 723 */
+ __u8 logical_block_size [ISODCL (129, 132)]; /* 723 */
+ __u8 path_table_size [ISODCL (133, 140)]; /* 733 */
+ __u8 type_l_path_table [ISODCL (141, 144)]; /* 731 */
+ __u8 opt_type_l_path_table [ISODCL (145, 148)]; /* 731 */
+ __u8 type_m_path_table [ISODCL (149, 152)]; /* 732 */
+ __u8 opt_type_m_path_table [ISODCL (153, 156)]; /* 732 */
+ __u8 root_directory_record [ISODCL (157, 190)]; /* 9.1 */
char volume_set_id [ISODCL (191, 318)]; /* dchars */
char publisher_id [ISODCL (319, 446)]; /* achars */
char preparer_id [ISODCL (447, 574)]; /* achars */
@@ -51,36 +51,36 @@ struct iso_primary_descriptor {
char copyright_file_id [ISODCL (703, 739)]; /* 7.5 dchars */
char abstract_file_id [ISODCL (740, 776)]; /* 7.5 dchars */
char bibliographic_file_id [ISODCL (777, 813)]; /* 7.5 dchars */
- char creation_date [ISODCL (814, 830)]; /* 8.4.26.1 */
- char modification_date [ISODCL (831, 847)]; /* 8.4.26.1 */
- char expiration_date [ISODCL (848, 864)]; /* 8.4.26.1 */
- char effective_date [ISODCL (865, 881)]; /* 8.4.26.1 */
- char file_structure_version [ISODCL (882, 882)]; /* 711 */
- char unused4 [ISODCL (883, 883)];
- char application_data [ISODCL (884, 1395)];
- char unused5 [ISODCL (1396, 2048)];
+ __u8 creation_date [ISODCL (814, 830)]; /* 8.4.26.1 */
+ __u8 modification_date [ISODCL (831, 847)]; /* 8.4.26.1 */
+ __u8 expiration_date [ISODCL (848, 864)]; /* 8.4.26.1 */
+ __u8 effective_date [ISODCL (865, 881)]; /* 8.4.26.1 */
+ __u8 file_structure_version [ISODCL (882, 882)]; /* 711 */
+ __u8 unused4 [ISODCL (883, 883)];
+ __u8 application_data [ISODCL (884, 1395)];
+ __u8 unused5 [ISODCL (1396, 2048)];
};

/* Almost the same as the primary descriptor but two fields are specified */
struct iso_supplementary_descriptor {
- char type [ISODCL ( 1, 1)]; /* 711 */
+ __u8 type [ISODCL ( 1, 1)]; /* 711 */
char id [ISODCL ( 2, 6)];
- char version [ISODCL ( 7, 7)]; /* 711 */
- char flags [ISODCL ( 8, 8)]; /* 853 */
+ __u8 version [ISODCL ( 7, 7)]; /* 711 */
+ __u8 flags [ISODCL ( 8, 8)]; /* 853 */
char system_id [ISODCL ( 9, 40)]; /* achars */
char volume_id [ISODCL ( 41, 72)]; /* dchars */
- char unused2 [ISODCL ( 73, 80)];
- char volume_space_size [ISODCL ( 81, 88)]; /* 733 */
- char escape [ISODCL ( 89, 120)]; /* 856 */
- char volume_set_size [ISODCL (121, 124)]; /* 723 */
- char volume_sequence_number [ISODCL (125, 128)]; /* 723 */
- char logical_block_size [ISODCL (129, 132)]; /* 723 */
- char path_table_size [ISODCL (133, 140)]; /* 733 */
- char type_l_path_table [ISODCL (141, 144)]; /* 731 */
- char opt_type_l_path_table [ISODCL (145, 148)]; /* 731 */
- char type_m_path_table [ISODCL (149, 152)]; /* 732 */
- char opt_type_m_path_table [ISODCL (153, 156)]; /* 732 */
- char root_directory_record [ISODCL (157, 190)]; /* 9.1 */
+ __u8 unused2 [ISODCL ( 73, 80)];
+ __u8 volume_space_size [ISODCL ( 81, 88)]; /* 733 */
+ __u8 escape [ISODCL ( 89, 120)]; /* 856 */
+ __u8 volume_set_size [ISODCL (121, 124)]; /* 723 */
+ __u8 volume_sequence_number [ISODCL (125, 128)]; /* 723 */
+ __u8 logical_block_size [ISODCL (129, 132)]; /* 723 */
+ __u8 path_table_size [ISODCL (133, 140)]; /* 733 */
+ __u8 type_l_path_table [ISODCL (141, 144)]; /* 731 */
+ __u8 opt_type_l_path_table [ISODCL (145, 148)]; /* 731 */
+ __u8 type_m_path_table [ISODCL (149, 152)]; /* 732 */
+ __u8 opt_type_m_path_table [ISODCL (153, 156)]; /* 732 */
+ __u8 root_directory_record [ISODCL (157, 190)]; /* 9.1 */
char volume_set_id [ISODCL (191, 318)]; /* dchars */
char publisher_id [ISODCL (319, 446)]; /* achars */
char preparer_id [ISODCL (447, 574)]; /* achars */
@@ -88,54 +88,54 @@ struct iso_supplementary_descriptor {
char copyright_file_id [ISODCL (703, 739)]; /* 7.5 dchars */
char abstract_file_id [ISODCL (740, 776)]; /* 7.5 dchars */
char bibliographic_file_id [ISODCL (777, 813)]; /* 7.5 dchars */
- char creation_date [ISODCL (814, 830)]; /* 8.4.26.1 */
- char modification_date [ISODCL (831, 847)]; /* 8.4.26.1 */
- char expiration_date [ISODCL (848, 864)]; /* 8.4.26.1 */
- char effective_date [ISODCL (865, 881)]; /* 8.4.26.1 */
- char file_structure_version [ISODCL (882, 882)]; /* 711 */
- char unused4 [ISODCL (883, 883)];
- char application_data [ISODCL (884, 1395)];
- char unused5 [ISODCL (1396, 2048)];
+ __u8 creation_date [ISODCL (814, 830)]; /* 8.4.26.1 */
+ __u8 modification_date [ISODCL (831, 847)]; /* 8.4.26.1 */
+ __u8 expiration_date [ISODCL (848, 864)]; /* 8.4.26.1 */
+ __u8 effective_date [ISODCL (865, 881)]; /* 8.4.26.1 */
+ __u8 file_structure_version [ISODCL (882, 882)]; /* 711 */
+ __u8 unused4 [ISODCL (883, 883)];
+ __u8 application_data [ISODCL (884, 1395)];
+ __u8 unused5 [ISODCL (1396, 2048)];
};

#define HS_STANDARD_ID "CDROM"

struct hs_volume_descriptor {
- char foo [ISODCL ( 1, 8)]; /* 733 */
- char type [ISODCL ( 9, 9)]; /* 711 */
+ __u8 foo [ISODCL ( 1, 8)]; /* 733 */
+ __u8 type [ISODCL ( 9, 9)]; /* 711 */
char id [ISODCL ( 10, 14)];
- char version [ISODCL ( 15, 15)]; /* 711 */
- char data[ISODCL(16,2048)];
+ __u8 version [ISODCL ( 15, 15)]; /* 711 */
+ __u8 data[ISODCL(16,2048)];
};

struct hs_primary_descriptor {
- char foo [ISODCL ( 1, 8)]; /* 733 */
- char type [ISODCL ( 9, 9)]; /* 711 */
- char id [ISODCL ( 10, 14)];
- char version [ISODCL ( 15, 15)]; /* 711 */
- char unused1 [ISODCL ( 16, 16)]; /* 711 */
+ __u8 foo [ISODCL ( 1, 8)]; /* 733 */
+ __u8 type [ISODCL ( 9, 9)]; /* 711 */
+ __u8 id [ISODCL ( 10, 14)];
+ __u8 version [ISODCL ( 15, 15)]; /* 711 */
+ __u8 unused1 [ISODCL ( 16, 16)]; /* 711 */
char system_id [ISODCL ( 17, 48)]; /* achars */
char volume_id [ISODCL ( 49, 80)]; /* dchars */
- char unused2 [ISODCL ( 81, 88)]; /* 733 */
- char volume_space_size [ISODCL ( 89, 96)]; /* 733 */
- char unused3 [ISODCL ( 97, 128)]; /* 733 */
- char volume_set_size [ISODCL (129, 132)]; /* 723 */
- char volume_sequence_number [ISODCL (133, 136)]; /* 723 */
- char logical_block_size [ISODCL (137, 140)]; /* 723 */
- char path_table_size [ISODCL (141, 148)]; /* 733 */
- char type_l_path_table [ISODCL (149, 152)]; /* 731 */
- char unused4 [ISODCL (153, 180)]; /* 733 */
- char root_directory_record [ISODCL (181, 214)]; /* 9.1 */
+ __u8 unused2 [ISODCL ( 81, 88)]; /* 733 */
+ __u8 volume_space_size [ISODCL ( 89, 96)]; /* 733 */
+ __u8 unused3 [ISODCL ( 97, 128)]; /* 733 */
+ __u8 volume_set_size [ISODCL (129, 132)]; /* 723 */
+ __u8 volume_sequence_number [ISODCL (133, 136)]; /* 723 */
+ __u8 logical_block_size [ISODCL (137, 140)]; /* 723 */
+ __u8 path_table_size [ISODCL (141, 148)]; /* 733 */
+ __u8 type_l_path_table [ISODCL (149, 152)]; /* 731 */
+ __u8 unused4 [ISODCL (153, 180)]; /* 733 */
+ __u8 root_directory_record [ISODCL (181, 214)]; /* 9.1 */
};

/* We use this to help us look up the parent inode numbers. */

struct iso_path_table{
- unsigned char name_len[2]; /* 721 */
- char extent[4]; /* 731 */
- char parent[2]; /* 721 */
+ __u8 name_len[2]; /* 721 */
+ __u8 extent[4]; /* 731 */
+ __u8 parent[2]; /* 721 */
char name[0];
} __attribute__((packed));

@@ -143,16 +143,16 @@ struct iso_path_table{
there is an extra reserved byte after the flags */

struct iso_directory_record {
- char length [ISODCL (1, 1)]; /* 711 */
- char ext_attr_length [ISODCL (2, 2)]; /* 711 */
- char extent [ISODCL (3, 10)]; /* 733 */
- char size [ISODCL (11, 18)]; /* 733 */
- char date [ISODCL (19, 25)]; /* 7 by 711 */
- char flags [ISODCL (26, 26)];
- char file_unit_size [ISODCL (27, 27)]; /* 711 */
- char interleave [ISODCL (28, 28)]; /* 711 */
- char volume_sequence_number [ISODCL (29, 32)]; /* 723 */
- unsigned char name_len [ISODCL (33, 33)]; /* 711 */
+ __u8 length [ISODCL (1, 1)]; /* 711 */
+ __u8 ext_attr_length [ISODCL (2, 2)]; /* 711 */
+ __u8 extent [ISODCL (3, 10)]; /* 733 */
+ __u8 size [ISODCL (11, 18)]; /* 733 */
+ __u8 date [ISODCL (19, 25)]; /* 7 by 711 */
+ __u8 flags [ISODCL (26, 26)];
+ __u8 file_unit_size [ISODCL (27, 27)]; /* 711 */
+ __u8 interleave [ISODCL (28, 28)]; /* 711 */
+ __u8 volume_sequence_number [ISODCL (29, 32)]; /* 723 */
+ __u8 name_len [ISODCL (33, 33)]; /* 711 */
char name [0];
} __attribute__((packed));

--
2.9.0

Next message: David Howells: "[PATCH 00/27] security, efi: Add kernel lockdown"
Previous message: Arnd Bergmann: "[PATCH v2 1/2] isofs: fix timestamps beyond 2027"
In reply to: Arnd Bergmann: "[PATCH v2 1/2] isofs: fix timestamps beyond 2027"
Next in thread: Jan Kara: "Re: [PATCH v2 1/2] isofs: fix timestamps beyond 2027"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]