Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753635Ab0HQBf3 (ORCPT ); Mon, 16 Aug 2010 21:35:29 -0400 Received: from mga11.intel.com ([192.55.52.93]:47678 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751898Ab0HQBf1 (ORCPT ); Mon, 16 Aug 2010 21:35:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.55,379,1278313200"; d="scan'208";a="597195592" Subject: Re: [PATCH 2/4] [Patch-next] ACPI, APEI, EINJ Fix the wrong checking of Injection Header's length From: Huang Ying To: Jin Dongming Cc: Randy Dunlap , Stephen Rothwell , Andi Kleen , Hidetoshi Seto , ACPI , LKLM In-Reply-To: <4C69DE4B.4090904@np.css.fujitsu.com> References: <4C69DE4B.4090904@np.css.fujitsu.com> Content-Type: text/plain; charset="UTF-8" Date: Tue, 17 Aug 2010 09:35:25 +0800 Message-ID: <1282008925.2744.1492.camel@yhuang-dev> Mime-Version: 1.0 X-Mailer: Evolution 2.30.2 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3569 Lines: 99 On Tue, 2010-08-17 at 08:56 +0800, Jin Dongming wrote: > header_length in struct acpi_table_einj is not the length of struct > acpi_table_einj, but the length of Injection Header. > > In einj_check_table(), header_length is used for checking the length > of struct acpi_table_einj. So I think it is wrong. Why "think"? Do you have a machine with header_length set as you said? > This patch fixed it and I confirmed it on x86_64 next-tree. > > Signed-off-by: Jin Dongming > --- > drivers/acpi/apei/einj.c | 15 +++++++++------ > include/acpi/actbl1.h | 17 ++++++++++++----- > 2 files changed, 21 insertions(+), 11 deletions(-) > > diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c > index 465c885..5fe876c 100644 > --- a/drivers/acpi/apei/einj.c > +++ b/drivers/acpi/apei/einj.c > @@ -104,7 +104,8 @@ static struct einj_parameter *einj_param; > static void einj_exec_ctx_init(struct apei_exec_context *ctx) > { > apei_exec_ctx_init(ctx, einj_ins_type, ARRAY_SIZE(einj_ins_type), > - EINJ_TAB_ENTRY(einj_tab), einj_tab->entries); > + EINJ_TAB_ENTRY(einj_tab), > + einj_tab->inje_header.entries); > } > > static int __einj_get_available_error_type(u32 *type) > @@ -153,7 +154,7 @@ static u64 einj_get_parameter_address(void) > struct acpi_whea_header *entry; > > entry = EINJ_TAB_ENTRY(einj_tab); > - for (i = 0; i < einj_tab->entries; i++) { > + for (i = 0; i < einj_tab->inje_header.entries; i++) { > if (entry->action == ACPI_EINJ_SET_ERROR_TYPE && > entry->instruction == ACPI_EINJ_WRITE_REGISTER && > entry->register_region.space_id == > @@ -426,12 +427,14 @@ DEFINE_SIMPLE_ATTRIBUTE(error_inject_fops, NULL, > > static int einj_check_table(struct acpi_table_einj *einj_tab) > { > - if (einj_tab->header_length != sizeof(struct acpi_table_einj)) > + if (einj_tab->common_header.length < sizeof(struct acpi_table_einj)) > return -EINVAL; > - if (einj_tab->header.length < sizeof(struct acpi_table_einj)) > + > + if (einj_tab->inje_header.length != sizeof(struct acpi_einj_header)) > return -EINVAL; > - if (einj_tab->entries != > - (einj_tab->header.length - sizeof(struct acpi_table_einj)) / > + > + if (einj_tab->inje_header.entries != > + (einj_tab->common_header.length - sizeof(struct acpi_table_einj)) / > sizeof(struct acpi_einj_entry)) > return -EINVAL; > > diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h > index 821f8ac..e586c30 100644 > --- a/include/acpi/actbl1.h > +++ b/include/acpi/actbl1.h > @@ -199,14 +199,21 @@ struct acpi_table_ecdt { > * > ******************************************************************************/ > > -struct acpi_table_einj { > - struct acpi_table_header header; /* Common ACPI table header */ > - u32 header_length; > - u8 flags; > - u8 reserved[3]; > +/* EINJ Injection Header */ > +struct acpi_einj_header { > + u32 length; > + u8 flags; > + u8 reserved[3]; > u32 entries; > }; > > +/* EINJ Header */ > + > +struct acpi_table_einj { > + struct acpi_table_header common_header; /* Common ACPI table header */ > + struct acpi_einj_header inje_header; /* Injection Header */ > +}; I don't think it is necessary to change the header definition, and inje_header is not a good name for me. Best Regards, Huang Ying -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/