|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] ACPI/ERST: fix signed/unsigned type conflicts
On 14.04.14 12:19, Jan Beulich wrote:
> 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@xxxxxxxx>
Full ok with me.
Reviewed-by: Christoph Egger <chegger@xxxxxxxxx>
>
> --- 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
> }
>
> /* 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 */
> --- 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));
> }
>
> /*
> --- 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_e
> 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 o
> 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
> 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 s
> 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 r
> 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_
> 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 rec
> 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,
> * 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 c
> * 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))
> --- 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);
> --- a/xen/include/asm-arm/types.h
> +++ b/xen/include/asm-arm/types.h
> @@ -56,6 +56,7 @@ typedef u64 register_t;
> #endif
>
> typedef unsigned long size_t;
> +typedef signed long ssize_t;
>
> typedef char bool_t;
> #define test_and_set_bool(b) xchg(&(b), 1)
> --- 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)
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |