ACPI/ERST: fix signed/unsigned type conflicts

Error checks exist in the respective code path that expect negative
values to indicate errors, yet negative values can't be communicated
through size_t. Thus ssize_t needs to be introduced (also on ARM for
consistency, even if the code in question isn't currently being used
on there).

The bug is theoretical only in so far as all the involved code is
effectively dead. Reflect this by excluding that code from non-debug
builds.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Christoph Egger <chegger@amazon.de>

diff --git a/xen/arch/x86/cpu/mcheck/mce-apei.c b/xen/arch/x86/cpu/mcheck/mce-apei.c
index 31f2302ed5af3e7f45c9ed96ba4ccc61c671dcdf..08f640105575ed80096ba9a0e2e5a13daba9aec9 100644 (file)
--- a/xen/arch/x86/cpu/mcheck/mce-apei.c
+++ b/xen/arch/x86/cpu/mcheck/mce-apei.c
@@ -89,10 +89,12 @@ int apei_write_mce(struct mce *m)
        return erst_write(&rcd.hdr);
 }
 
-size_t apei_read_mce(struct mce *m, u64 *record_id)
+#ifndef NDEBUG /* currently dead code */
+
+ssize_t apei_read_mce(struct mce *m, u64 *record_id)
 {
        struct cper_mce_record rcd;
-       size_t len;
+       ssize_t len;
 
        if (!m || !record_id)
                return -EINVAL;
@@ -115,12 +117,14 @@ size_t apei_read_mce(struct mce *m, u64 *record_id)
 }
 
 /* Check whether there is record in ERST */
-int apei_check_mce(void)
+bool_t apei_check_mce(void)
 {
-       return erst_get_record_count();
+       return erst_get_record_count() > 0;
 }
 
 int apei_clear_mce(u64 record_id)
 {
        return erst_clear(record_id);
 }
+
+#endif /* currently dead code */

diff --git a/xen/common/lib.c b/xen/common/lib.c
index e3cb3f4730ef00fd5f7de6b8083979c7579e3a62..a7580259643951e780925534046369f8588b1b50 100644 (file)
--- a/xen/common/lib.c
+++ b/xen/common/lib.c
@@ -487,6 +487,9 @@ void __init init_constructors(void)
     const ctor_func_t *f;
     for ( f = __ctors_start; f < __ctors_end; ++f )
         (*f)();
+
+    /* Putting this here seems as good (or bad) as any other place. */
+    BUILD_BUG_ON(sizeof(size_t) != sizeof(ssize_t));
 }
 
 /*

diff --git a/xen/drivers/acpi/apei/erst.c b/xen/drivers/acpi/apei/erst.c
index 98f73d906154274a835dc740b6ade895aa1b8423..2fafe7fcf728c7d6ea1e47fc387e3af789da5873 100644 (file)
--- a/xen/drivers/acpi/apei/erst.c
+++ b/xen/drivers/acpi/apei/erst.c
@@ -383,21 +383,24 @@ static int erst_get_erange(struct erst_erange *range)
        return 0;
 }
 
-static size_t __erst_get_record_count(void)
+static ssize_t __erst_get_record_count(void)
 {
        struct apei_exec_context ctx;
        int rc;
+       u64 output;
+       ssize_t count;
 
        erst_exec_ctx_init(&ctx);
        rc = apei_exec_run(&ctx, ACPI_ERST_GET_RECORD_COUNT);
        if (rc)
                return rc;
-       return apei_exec_ctx_get_output(&ctx);
+       count = output = apei_exec_ctx_get_output(&ctx);
+       return count >= 0 && count == output ? count : -ERANGE;
 }
 
-size_t erst_get_record_count(void)
+ssize_t erst_get_record_count(void)
 {
-       size_t count;
+       ssize_t count;
        unsigned long flags;
 
        if (!erst_enabled)
@@ -483,6 +486,8 @@ static int __erst_write_to_storage(u64 offset)
        return erst_errno(val);
 }
 
+#ifndef NDEBUG /* currently dead code */
+
 static int __erst_read_from_storage(u64 record_id, u64 offset)
 {
        struct apei_exec_context ctx;
@@ -565,6 +570,8 @@ static int __erst_clear_from_storage(u64 record_id)
        return erst_errno(val);
 }
 
+#endif /* currently dead code */
+
 /* NVRAM ERST Error Log Address Range is not supported yet */
 static int __erst_write_to_nvram(const struct cper_record_header *record)
 {
@@ -572,6 +579,8 @@ static int __erst_write_to_nvram(const struct cper_record_header *record)
        return -ENOSYS;
 }
 
+#ifndef NDEBUG /* currently dead code */
+
 static int __erst_read_to_erange_from_nvram(u64 record_id, u64 *offset)
 {
        printk(KERN_WARNING
@@ -586,6 +595,8 @@ static int __erst_clear_from_nvram(u64 record_id)
        return -ENOSYS;
 }
 
+#endif /* currently dead code */
+
 int erst_write(const struct cper_record_header *record)
 {
        int rc;
@@ -625,6 +636,8 @@ int erst_write(const struct cper_record_header *record)
        return rc;
 }
 
+#ifndef NDEBUG /* currently dead code */
+
 static int __erst_read_to_erange(u64 record_id, u64 *offset)
 {
        int rc;
@@ -641,22 +654,24 @@ static int __erst_read_to_erange(u64 record_id, u64 *offset)
        return 0;
 }
 
-static size_t __erst_read(u64 record_id, struct cper_record_header *record,
+static ssize_t __erst_read(u64 record_id, struct cper_record_header *record,
                           size_t buflen)
 {
        int rc;
-       u64 offset, len = 0;
+       u64 offset;
+       ssize_t len;
        struct cper_record_header *rcd_tmp;
 
        rc = __erst_read_to_erange(record_id, &offset);
        if (rc)
                return rc;
        rcd_tmp = erst_erange.vaddr + offset;
+       if (rcd_tmp->record_length > buflen)
+               return -ENOBUFS;
        len = rcd_tmp->record_length;
-       if (len <= buflen)
-               memcpy(record, rcd_tmp, len);
+       memcpy(record, rcd_tmp, len);
 
-       return len;
+       return len >= 0 ? len : -ERANGE;
 }
 
 /*
@@ -664,10 +679,10 @@ static size_t __erst_read(u64 record_id, struct cper_record_header *record,
  * else if return value < 0, something goes wrong,
  * else everything is OK, and return value is record length
  */
-size_t erst_read(u64 record_id, struct cper_record_header *record,
+ssize_t erst_read(u64 record_id, struct cper_record_header *record,
                  size_t buflen)
 {
-       size_t len;
+       ssize_t len;
        unsigned long flags;
 
        if (!erst_enabled)
@@ -685,10 +700,10 @@ size_t erst_read(u64 record_id, struct cper_record_header *record,
  * else if return value < 0, something goes wrong,
  * else everything is OK, and return value is record length
  */
-size_t erst_read_next(struct cper_record_header *record, size_t buflen)
+ssize_t erst_read_next(struct cper_record_header *record, size_t buflen)
 {
        int rc;
-       size_t len;
+       ssize_t len;
        unsigned long flags;
        u64 record_id;
 
@@ -731,6 +746,8 @@ int erst_clear(u64 record_id)
        return rc;
 }
 
+#endif /* currently dead code */
+
 static int __init erst_check_table(struct acpi_table_erst *erst_tab)
 {
        if (erst_tab->header.length < sizeof(*erst_tab))

diff --git a/xen/include/acpi/apei.h b/xen/include/acpi/apei.h
index 162f616ef0ea995590c0ad458d5519431eca692a..d22859570867b5235b96d8908e76ab8854927153 100644 (file)
--- a/xen/include/acpi/apei.h
+++ b/xen/include/acpi/apei.h
@@ -13,11 +13,11 @@
 #define FIX_APEI_RANGE_MAX 64
 
 int erst_write(const struct cper_record_header *record);
-size_t erst_get_record_count(void);
+ssize_t erst_get_record_count(void);
 int erst_get_next_record_id(u64 *record_id);
-size_t erst_read(u64 record_id, struct cper_record_header *record,
+ssize_t erst_read(u64 record_id, struct cper_record_header *record,
                  size_t buflen);
-size_t erst_read_next(struct cper_record_header *record, size_t buflen);
+ssize_t erst_read_next(struct cper_record_header *record, size_t buflen);
 int erst_clear(u64 record_id);
 
 void __iomem *apei_pre_map(paddr_t paddr, unsigned long size);

diff --git a/xen/include/asm-arm/types.h b/xen/include/asm-arm/types.h
index 183f3e073351cc56b51f928216a8f1ab79d43207..09e54553645f62d8bcc56b48ec8b352da80de4bb 100644 (file)
--- a/xen/include/asm-arm/types.h
+++ b/xen/include/asm-arm/types.h
@@ -60,6 +60,7 @@ typedef __SIZE_TYPE__ size_t;
 #else
 typedef unsigned long size_t;
 #endif
+typedef signed long ssize_t;
 
 typedef char bool_t;
 #define test_and_set_bool(b)   xchg(&(b), 1)

diff --git a/xen/include/asm-x86/types.h b/xen/include/asm-x86/types.h
index 9da305346b08181c2f66c7c3887b9028f59fcfd1..b82fa58aa095f7dd846e46ae961b520d6d7c6060 100644 (file)
--- a/xen/include/asm-x86/types.h
+++ b/xen/include/asm-x86/types.h
@@ -39,6 +39,7 @@ typedef __SIZE_TYPE__ size_t;
 #else
 typedef unsigned long size_t;
 #endif
+typedef signed long ssize_t;
 
 typedef char bool_t;
 #define test_and_set_bool(b)   xchg(&(b), 1)