Skip to contentSkip to conversation

efi: move some sysfs files to be read-only by root

PATCH Superseded Greg Kroah-Hartman 3 participants 7 messages Dec 5, 2017 – Dec 6, 2017

Subsystems (2)
lore
Share & tools
lore

Greg Kroah-Hartman

author maintainer
#0 lore raw

PATCH 1/1 efi: move some sysfs files to be read-only by root

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    5 +++++
 4 files changed, 17 insertions(+), 16 deletions(-)
drivers/firmware/efi/efi.c +1 -2
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
 	return str - buf;
 }
 
-static struct kobj_attribute efi_attr_systab =
-			__ATTR(systab, 0400, systab_show, NULL);
+static struct kobj_attribute efi_attr_systab = __ATTR_RO_MODE(systab, 0400);
 
 #define EFI_FIELD(var) efi.var
 
drivers/firmware/efi/esrt.c +6 -9
@@ -106,7 +106,7 @@ static const struct sysfs_ops esre_attr_
 };
 
 /* Generic ESRT Entry ("ESRE") support. */
-static ssize_t esre_fw_class_show(struct esre_entry *entry, char *buf)
+static ssize_t fw_class_show(struct esre_entry *entry, char *buf)
 {
 	char *str = buf;
 
@@ -117,18 +117,16 @@ static ssize_t esre_fw_class_show(struct
 	return str - buf;
 }
 
-static struct esre_attribute esre_fw_class = __ATTR(fw_class, 0400,
-	esre_fw_class_show, NULL);
+static struct esre_attribute esre_fw_class = __ATTR_RO_MODE(fw_class, 0400);
 
 #define esre_attr_decl(name, size, fmt) \
-static ssize_t esre_##name##_show(struct esre_entry *entry, char *buf) \
+static ssize_t name##_show(struct esre_entry *entry, char *buf) \
 { \
 	return sprintf(buf, fmt "\n", \
 		       le##size##_to_cpu(entry->esre.esre1->name)); \
 } \
 \
-static struct esre_attribute esre_##name = __ATTR(name, 0400, \
-	esre_##name##_show, NULL)
+static struct esre_attribute esre_##name = __ATTR_RO_MODE(name, 0400)
 
 esre_attr_decl(fw_type, 32, "%u");
 esre_attr_decl(fw_version, 32, "%u");
@@ -193,14 +191,13 @@ static int esre_create_sysfs_entry(void
 
 /* support for displaying ESRT fields at the top level */
 #define esrt_attr_decl(name, size, fmt) \
-static ssize_t esrt_##name##_show(struct kobject *kobj, \
+static ssize_t name##_show(struct kobject *kobj, \
 				  struct kobj_attribute *attr, char *buf)\
 { \
 	return sprintf(buf, fmt "\n", le##size##_to_cpu(esrt->name)); \
 } \
 \
-static struct kobj_attribute esrt_##name = __ATTR(name, 0400, \
-	esrt_##name##_show, NULL)
+static struct kobj_attribute esrt_##name = __ATTR_RO_MODE(name, 0400)
 
 esrt_attr_decl(fw_resource_count, 32, "%u");
 esrt_attr_decl(fw_resource_count_max, 32, "%u");
drivers/firmware/efi/runtime-map.c +5 -5
@@ -63,11 +63,11 @@ static ssize_t map_attr_show(struct kobj
 	return map_attr->show(entry, buf);
 }
 
-static struct map_attribute map_type_attr = __ATTR_RO(type);
-static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
-static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
-static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
-static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+static struct map_attribute map_type_attr = __ATTR_RO_MODE(type, 0400);
+static struct map_attribute map_phys_addr_attr = __ATTR_RO_MODE(phys_addr, 0400);
+static struct map_attribute map_virt_addr_attr = __ATTR_RO_MODE(virt_addr, 0400);
+static struct map_attribute map_num_pages_attr = __ATTR_RO_MODE(num_pages, 0400);
+static struct map_attribute map_attribute_attr = __ATTR_RO_MODE(attribute, 0400);
 
 /*
  * These are default attributes that are added for every memmap entry.
include/linux/sysfs.h +5 -0
@@ -117,6 +117,11 @@ struct attribute_group {
 	.show	= _name##_show,						\
 }
 
+#define __ATTR_RO_MODE(_name, _mode) {					\
+	.attr	= { .name = __stringify(_name), .mode = _mode },	\
+	.show	= _name##_show,						\
+}
+
 #define __ATTR_WO(_name) {						\
 	.attr	= { .name = __stringify(_name), .mode = S_IWUSR },	\
 	.store	= _name##_store,					\
#1 lore raw
Quoted 106 lines · Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    5 +++++
 4 files changed, 17 insertions(+), 16 deletions(-)
drivers/firmware/efi/efi.c +1 -2
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
        return str - buf;
 }
 
-static struct kobj_attribute efi_attr_systab =
-                       __ATTR(systab, 0400, systab_show, NULL);
+static struct kobj_attribute efi_attr_systab = __ATTR_RO_MODE(systab, 0400);
 
 #define EFI_FIELD(var) efi.var
 
drivers/firmware/efi/esrt.c +6 -9
@@ -106,7 +106,7 @@ static const struct sysfs_ops esre_attr_
 };
 
 /* Generic ESRT Entry ("ESRE") support. */
-static ssize_t esre_fw_class_show(struct esre_entry *entry, char *buf)
+static ssize_t fw_class_show(struct esre_entry *entry, char *buf)
 {
        char *str = buf;
 
@@ -117,18 +117,16 @@ static ssize_t esre_fw_class_show(struct
        return str - buf;
 }
 
-static struct esre_attribute esre_fw_class = __ATTR(fw_class, 0400,
-       esre_fw_class_show, NULL);
+static struct esre_attribute esre_fw_class = __ATTR_RO_MODE(fw_class, 0400);
 
 #define esre_attr_decl(name, size, fmt) \
-static ssize_t esre_##name##_show(struct esre_entry *entry, char *buf) \
+static ssize_t name##_show(struct esre_entry *entry, char *buf) \
 { \
        return sprintf(buf, fmt "\n", \
                       le##size##_to_cpu(entry->esre.esre1->name)); \
 } \
 \
-static struct esre_attribute esre_##name = __ATTR(name, 0400, \
-       esre_##name##_show, NULL)
+static struct esre_attribute esre_##name = __ATTR_RO_MODE(name, 0400)
 
 esre_attr_decl(fw_type, 32, "%u");
 esre_attr_decl(fw_version, 32, "%u");
@@ -193,14 +191,13 @@ static int esre_create_sysfs_entry(void
 
 /* support for displaying ESRT fields at the top level */
 #define esrt_attr_decl(name, size, fmt) \
-static ssize_t esrt_##name##_show(struct kobject *kobj, \
+static ssize_t name##_show(struct kobject *kobj, \
                                  struct kobj_attribute *attr, char *buf)\
 { \
        return sprintf(buf, fmt "\n", le##size##_to_cpu(esrt->name)); \
 } \
 \
-static struct kobj_attribute esrt_##name = __ATTR(name, 0400, \
-       esrt_##name##_show, NULL)
+static struct kobj_attribute esrt_##name = __ATTR_RO_MODE(name, 0400)
 
 esrt_attr_decl(fw_resource_count, 32, "%u");
 esrt_attr_decl(fw_resource_count_max, 32, "%u");
drivers/firmware/efi/runtime-map.c +5 -5
@@ -63,11 +63,11 @@ static ssize_t map_attr_show(struct kobj
        return map_attr->show(entry, buf);
 }
 
-static struct map_attribute map_type_attr = __ATTR_RO(type);
-static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
-static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
-static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
-static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+static struct map_attribute map_type_attr = __ATTR_RO_MODE(type, 0400);
+static struct map_attribute map_phys_addr_attr = __ATTR_RO_MODE(phys_addr, 0400);
+static struct map_attribute map_virt_addr_attr = __ATTR_RO_MODE(virt_addr, 0400);
+static struct map_attribute map_num_pages_attr = __ATTR_RO_MODE(num_pages, 0400);
+static struct map_attribute map_attribute_attr = __ATTR_RO_MODE(attribute, 0400);
 
 /*
  * These are default attributes that are added for every memmap entry.
include/linux/sysfs.h +0 -0
@@ -117,6 +117,11 @@ struct attribute_group {
        .show   = _name##_show,                                         \

include/linux/sysfs.h struct attribute_group { · line 127

Quoted 10 lines
include/linux/sysfs.h +5 -0
 }
 
+#define __ATTR_RO_MODE(_name, _mode) {                                 \
+       .attr   = { .name = __stringify(_name), .mode = _mode },        \
+       .show   = _name##_show,                                         \
+}
+
 #define __ATTR_WO(_name) {                                             \
        .attr   = { .name = __stringify(_name), .mode = S_IWUSR },      \
        .store  = _name##_store,                                        \

Thanks Greg.

Do we need the VERIFY_OCTAL_PERMISSION() thing here as well?

author maintainer
#2 lore raw
Quoted 121 lines · Ard Biesheuvel
Quoted 106 lines · Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    5 +++++
 4 files changed, 17 insertions(+), 16 deletions(-)
drivers/firmware/efi/efi.c +1 -2
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
        return str - buf;
 }
 
-static struct kobj_attribute efi_attr_systab =
-                       __ATTR(systab, 0400, systab_show, NULL);
+static struct kobj_attribute efi_attr_systab = __ATTR_RO_MODE(systab, 0400);
 
 #define EFI_FIELD(var) efi.var
 
drivers/firmware/efi/esrt.c +6 -9
@@ -106,7 +106,7 @@ static const struct sysfs_ops esre_attr_
 };
 
 /* Generic ESRT Entry ("ESRE") support. */
-static ssize_t esre_fw_class_show(struct esre_entry *entry, char *buf)
+static ssize_t fw_class_show(struct esre_entry *entry, char *buf)
 {
        char *str = buf;
 
@@ -117,18 +117,16 @@ static ssize_t esre_fw_class_show(struct
        return str - buf;
 }
 
-static struct esre_attribute esre_fw_class = __ATTR(fw_class, 0400,
-       esre_fw_class_show, NULL);
+static struct esre_attribute esre_fw_class = __ATTR_RO_MODE(fw_class, 0400);
 
 #define esre_attr_decl(name, size, fmt) \
-static ssize_t esre_##name##_show(struct esre_entry *entry, char *buf) \
+static ssize_t name##_show(struct esre_entry *entry, char *buf) \
 { \
        return sprintf(buf, fmt "\n", \
                       le##size##_to_cpu(entry->esre.esre1->name)); \
 } \
 \
-static struct esre_attribute esre_##name = __ATTR(name, 0400, \
-       esre_##name##_show, NULL)
+static struct esre_attribute esre_##name = __ATTR_RO_MODE(name, 0400)
 
 esre_attr_decl(fw_type, 32, "%u");
 esre_attr_decl(fw_version, 32, "%u");
@@ -193,14 +191,13 @@ static int esre_create_sysfs_entry(void
 
 /* support for displaying ESRT fields at the top level */
 #define esrt_attr_decl(name, size, fmt) \
-static ssize_t esrt_##name##_show(struct kobject *kobj, \
+static ssize_t name##_show(struct kobject *kobj, \
                                  struct kobj_attribute *attr, char *buf)\
 { \
        return sprintf(buf, fmt "\n", le##size##_to_cpu(esrt->name)); \
 } \
 \
-static struct kobj_attribute esrt_##name = __ATTR(name, 0400, \
-       esrt_##name##_show, NULL)
+static struct kobj_attribute esrt_##name = __ATTR_RO_MODE(name, 0400)
 
 esrt_attr_decl(fw_resource_count, 32, "%u");
 esrt_attr_decl(fw_resource_count_max, 32, "%u");
drivers/firmware/efi/runtime-map.c +5 -5
@@ -63,11 +63,11 @@ static ssize_t map_attr_show(struct kobj
        return map_attr->show(entry, buf);
 }
 
-static struct map_attribute map_type_attr = __ATTR_RO(type);
-static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
-static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
-static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
-static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+static struct map_attribute map_type_attr = __ATTR_RO_MODE(type, 0400);
+static struct map_attribute map_phys_addr_attr = __ATTR_RO_MODE(phys_addr, 0400);
+static struct map_attribute map_virt_addr_attr = __ATTR_RO_MODE(virt_addr, 0400);
+static struct map_attribute map_num_pages_attr = __ATTR_RO_MODE(num_pages, 0400);
+static struct map_attribute map_attribute_attr = __ATTR_RO_MODE(attribute, 0400);
 
 /*
  * These are default attributes that are added for every memmap entry.
include/linux/sysfs.h +0 -0
@@ -117,6 +117,11 @@ struct attribute_group {
        .show   = _name##_show,                                         \

include/linux/sysfs.h struct attribute_group { · line 127

Quoted 10 lines
include/linux/sysfs.h +5 -0
 }
 
+#define __ATTR_RO_MODE(_name, _mode) {                                 \
+       .attr   = { .name = __stringify(_name), .mode = _mode },        \
+       .show   = _name##_show,                                         \
+}
+
 #define __ATTR_WO(_name) {                                             \
        .attr   = { .name = __stringify(_name), .mode = S_IWUSR },      \
        .store  = _name##_store,                                        \

Thanks Greg.

Quoted 1 line

Do we need the VERIFY_OCTAL_PERMISSION() thing here as well?

Ah, totally missed that, let me go add it now, good catch...

greg k-h

author maintainer
#3 lore raw

PATCH v2 1/1 efi: move some sysfs files to be read-only by root

In reply to Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
v2: add VERIFY_OCTAL_PERMISSIONS to __ATTR_RO_MODE() as pointed out by
Ard.

 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    6 ++++++
 4 files changed, 18 insertions(+), 16 deletions(-)
drivers/firmware/efi/efi.c +1 -2
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
 	return str - buf;
 }
 
-static struct kobj_attribute efi_attr_systab =
-			__ATTR(systab, 0400, systab_show, NULL);
+static struct kobj_attribute efi_attr_systab = __ATTR_RO_MODE(systab, 0400);
 
 #define EFI_FIELD(var) efi.var
 
drivers/firmware/efi/esrt.c +6 -9
@@ -106,7 +106,7 @@ static const struct sysfs_ops esre_attr_
 };
 
 /* Generic ESRT Entry ("ESRE") support. */
-static ssize_t esre_fw_class_show(struct esre_entry *entry, char *buf)
+static ssize_t fw_class_show(struct esre_entry *entry, char *buf)
 {
 	char *str = buf;
 
@@ -117,18 +117,16 @@ static ssize_t esre_fw_class_show(struct
 	return str - buf;
 }
 
-static struct esre_attribute esre_fw_class = __ATTR(fw_class, 0400,
-	esre_fw_class_show, NULL);
+static struct esre_attribute esre_fw_class = __ATTR_RO_MODE(fw_class, 0400);
 
 #define esre_attr_decl(name, size, fmt) \
-static ssize_t esre_##name##_show(struct esre_entry *entry, char *buf) \
+static ssize_t name##_show(struct esre_entry *entry, char *buf) \
 { \
 	return sprintf(buf, fmt "\n", \
 		       le##size##_to_cpu(entry->esre.esre1->name)); \
 } \
 \
-static struct esre_attribute esre_##name = __ATTR(name, 0400, \
-	esre_##name##_show, NULL)
+static struct esre_attribute esre_##name = __ATTR_RO_MODE(name, 0400)
 
 esre_attr_decl(fw_type, 32, "%u");
 esre_attr_decl(fw_version, 32, "%u");
@@ -193,14 +191,13 @@ static int esre_create_sysfs_entry(void
 
 /* support for displaying ESRT fields at the top level */
 #define esrt_attr_decl(name, size, fmt) \
-static ssize_t esrt_##name##_show(struct kobject *kobj, \
+static ssize_t name##_show(struct kobject *kobj, \
 				  struct kobj_attribute *attr, char *buf)\
 { \
 	return sprintf(buf, fmt "\n", le##size##_to_cpu(esrt->name)); \
 } \
 \
-static struct kobj_attribute esrt_##name = __ATTR(name, 0400, \
-	esrt_##name##_show, NULL)
+static struct kobj_attribute esrt_##name = __ATTR_RO_MODE(name, 0400)
 
 esrt_attr_decl(fw_resource_count, 32, "%u");
 esrt_attr_decl(fw_resource_count_max, 32, "%u");
drivers/firmware/efi/runtime-map.c +5 -5
@@ -63,11 +63,11 @@ static ssize_t map_attr_show(struct kobj
 	return map_attr->show(entry, buf);
 }
 
-static struct map_attribute map_type_attr = __ATTR_RO(type);
-static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
-static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
-static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
-static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+static struct map_attribute map_type_attr = __ATTR_RO_MODE(type, 0400);
+static struct map_attribute map_phys_addr_attr = __ATTR_RO_MODE(phys_addr, 0400);
+static struct map_attribute map_virt_addr_attr = __ATTR_RO_MODE(virt_addr, 0400);
+static struct map_attribute map_num_pages_attr = __ATTR_RO_MODE(num_pages, 0400);
+static struct map_attribute map_attribute_attr = __ATTR_RO_MODE(attribute, 0400);
 
 /*
  * These are default attributes that are added for every memmap entry.
include/linux/sysfs.h +6 -0
@@ -117,6 +117,12 @@ struct attribute_group {
 	.show	= _name##_show,						\
 }
 
+#define __ATTR_RO_MODE(_name, _mode) {					\
+	.attr	= { .name = __stringify(_name),				\
+		    .mode = VERIFY_OCTAL_PERMISSIONS(_mode) },		\
+	.show	= _name##_show,						\
+}
+
 #define __ATTR_WO(_name) {						\
 	.attr	= { .name = __stringify(_name), .mode = S_IWUSR },	\
 	.store	= _name##_store,					\
#4 lore raw
Quoted 13 lines · Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

Quoted 4 lines

---
v2: add VERIFY_OCTAL_PERMISSIONS to __ATTR_RO_MODE() as pointed out by
Ard.

Thanks. I will queue this as a fix in the EFI tree (as well as Dave's patch that adds the systab comment)

Quoted 106 lines
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    6 ++++++
 4 files changed, 18 insertions(+), 16 deletions(-)
drivers/firmware/efi/efi.c +1 -2
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
        return str - buf;
 }
 
-static struct kobj_attribute efi_attr_systab =
-                       __ATTR(systab, 0400, systab_show, NULL);
+static struct kobj_attribute efi_attr_systab = __ATTR_RO_MODE(systab, 0400);
 
 #define EFI_FIELD(var) efi.var
 
drivers/firmware/efi/esrt.c +6 -9
@@ -106,7 +106,7 @@ static const struct sysfs_ops esre_attr_
 };
 
 /* Generic ESRT Entry ("ESRE") support. */
-static ssize_t esre_fw_class_show(struct esre_entry *entry, char *buf)
+static ssize_t fw_class_show(struct esre_entry *entry, char *buf)
 {
        char *str = buf;
 
@@ -117,18 +117,16 @@ static ssize_t esre_fw_class_show(struct
        return str - buf;
 }
 
-static struct esre_attribute esre_fw_class = __ATTR(fw_class, 0400,
-       esre_fw_class_show, NULL);
+static struct esre_attribute esre_fw_class = __ATTR_RO_MODE(fw_class, 0400);
 
 #define esre_attr_decl(name, size, fmt) \
-static ssize_t esre_##name##_show(struct esre_entry *entry, char *buf) \
+static ssize_t name##_show(struct esre_entry *entry, char *buf) \
 { \
        return sprintf(buf, fmt "\n", \
                       le##size##_to_cpu(entry->esre.esre1->name)); \
 } \
 \
-static struct esre_attribute esre_##name = __ATTR(name, 0400, \
-       esre_##name##_show, NULL)
+static struct esre_attribute esre_##name = __ATTR_RO_MODE(name, 0400)
 
 esre_attr_decl(fw_type, 32, "%u");
 esre_attr_decl(fw_version, 32, "%u");
@@ -193,14 +191,13 @@ static int esre_create_sysfs_entry(void
 
 /* support for displaying ESRT fields at the top level */
 #define esrt_attr_decl(name, size, fmt) \
-static ssize_t esrt_##name##_show(struct kobject *kobj, \
+static ssize_t name##_show(struct kobject *kobj, \
                                  struct kobj_attribute *attr, char *buf)\
 { \
        return sprintf(buf, fmt "\n", le##size##_to_cpu(esrt->name)); \
 } \
 \
-static struct kobj_attribute esrt_##name = __ATTR(name, 0400, \
-       esrt_##name##_show, NULL)
+static struct kobj_attribute esrt_##name = __ATTR_RO_MODE(name, 0400)
 
 esrt_attr_decl(fw_resource_count, 32, "%u");
 esrt_attr_decl(fw_resource_count_max, 32, "%u");
drivers/firmware/efi/runtime-map.c +5 -5
@@ -63,11 +63,11 @@ static ssize_t map_attr_show(struct kobj
        return map_attr->show(entry, buf);
 }
 
-static struct map_attribute map_type_attr = __ATTR_RO(type);
-static struct map_attribute map_phys_addr_attr   = __ATTR_RO(phys_addr);
-static struct map_attribute map_virt_addr_attr  = __ATTR_RO(virt_addr);
-static struct map_attribute map_num_pages_attr  = __ATTR_RO(num_pages);
-static struct map_attribute map_attribute_attr  = __ATTR_RO(attribute);
+static struct map_attribute map_type_attr = __ATTR_RO_MODE(type, 0400);
+static struct map_attribute map_phys_addr_attr = __ATTR_RO_MODE(phys_addr, 0400);
+static struct map_attribute map_virt_addr_attr = __ATTR_RO_MODE(virt_addr, 0400);
+static struct map_attribute map_num_pages_attr = __ATTR_RO_MODE(num_pages, 0400);
+static struct map_attribute map_attribute_attr = __ATTR_RO_MODE(attribute, 0400);
 
 /*
  * These are default attributes that are added for every memmap entry.
include/linux/sysfs.h +6 -1
@@ -117,6 +117,12 @@ struct attribute_group {
        .show   = _name##_show,                                         \
 }
 
+#define __ATTR_RO_MODE(_name, _mode) {                                 \
+       .attr   = { .name = __stringify(_name),                         \
+                   .mode = VERIFY_OCTAL_PERMISSIONS(_mode) },          \
+       .show   = _name##_show,                                         \
+}
+
 #define __ATTR_WO(_name) {                                             \
        .attr   = { .name = __stringify(_name), .mode = S_IWUSR },      \
        .store  = _name##_store,                                        \
--

To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw
In reply to Greg Kroah-Hartman
Quoted 23 lines · Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    5 +++++
 4 files changed, 17 insertions(+), 16 deletions(-)

drivers/firmware/efi/efi.c static ssize_t systab_show(struct kobjec · line 144

Quoted 2 lines
drivers/firmware/efi/efi.c +0 -0
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
 	return str - buf;
 }

Greg, do you add the CC's here in the commit log for a technical reason? Is it so that future investigation that leads to this commit can see who to involve in any further discussion?

As an example, for the patch that added the %p hashing should I have CC'd Jason A. Donenfeld since he was the brains behind the SipHash stuff and gave loads of suggestions/direction?

thanks,
Tobin.

author maintainer
#6 lore raw
Quoted 21 lines · Tobin C. Harding
Quoted 20 lines · Greg Kroah-Hartman

Thanks to the scripts/leaking_addresses.pl script, it was found that some EFI values should not be readable by non-root users.

So make them root-only, and to do that, add a __ATTR_RO_MODE() macro to make this easier, and use it in other places at the same time.

---
 drivers/firmware/efi/efi.c         |    3 +--
 drivers/firmware/efi/esrt.c        |   15 ++++++---------
 drivers/firmware/efi/runtime-map.c |   10 +++++-----
 include/linux/sysfs.h              |    5 +++++
 4 files changed, 17 insertions(+), 16 deletions(-)
Quoted 9 lines

drivers/firmware/efi/efi.c static ssize_t systab_show(struct kobjec · line 144

Quoted 5 lines
drivers/firmware/efi/efi.c +0 -0
@@ -143,8 +143,7 @@ static ssize_t systab_show(struct kobjec
 	return str - buf;
 }

Greg, do you add the CC's here in the commit log for a technical reason? Is it so that future investigation that leads to this commit can see who to involve in any further discussion?

They came from the output of scripts/get_maintainer.pl on who I should be sending the patch to, and who should hopefully review it.

Quoted 3 lines

As an example, for the patch that added the %p hashing should I have CC'd Jason A. Donenfeld since he was the brains behind the SipHash stuff and gave loads of suggestions/direction?

If you want to. It's also a good way for me to track who the patch gets sent to when doing multiple versions of a patch series. git send-email picks those up and sends the patch to them as well, making it easier on the developer instead of having to remember a long --cc= list of addresses.

thanks,

greg k-h