* [PATCH] fs: partitions: efi: Fix bound check
@ 2013-11-15 17:14 Antti P Miettinen
2013-11-20 0:04 ` Andrew Morton
2013-11-20 1:06 ` Davidlohr Bueso
0 siblings, 2 replies; 6+ messages in thread
From: Antti P Miettinen @ 2013-11-15 17:14 UTC (permalink / raw)
To: axboe, akpm, davidlohr, matt.fleming, kzak; +Cc: linux-kernel
Use ARRAY_SIZE instead of sizeof to get proper max for label
length.
Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
---
block/partitions/efi.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index a8287b4..dc51f46 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -96,6 +96,7 @@
* - Code works, detects all the partitions.
*
************************************************************/
+#include <linux/kernel.h>
#include <linux/crc32.h>
#include <linux/ctype.h>
#include <linux/math64.h>
@@ -715,8 +716,8 @@ int efi_partition(struct parsed_partitions *state)
efi_guid_unparse(&ptes[i].unique_partition_guid, info->uuid);
/* Naively convert UTF16-LE to 7 bits. */
- label_max = min(sizeof(info->volname) - 1,
- sizeof(ptes[i].partition_name));
+ label_max = min(ARRAY_SIZE(info->volname) - 1,
+ ARRAY_SIZE(ptes[i].partition_name));
info->volname[label_max] = 0;
while (label_count < label_max) {
u8 c = ptes[i].partition_name[label_count] & 0xff;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: partitions: efi: Fix bound check
2013-11-15 17:14 [PATCH] fs: partitions: efi: Fix bound check Antti P Miettinen
@ 2013-11-20 0:04 ` Andrew Morton
2013-11-20 7:18 ` Antti Miettinen
2013-11-20 1:06 ` Davidlohr Bueso
1 sibling, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2013-11-20 0:04 UTC (permalink / raw)
To: amiettinen; +Cc: axboe, davidlohr, matt.fleming, kzak, linux-kernel
On Fri, 15 Nov 2013 19:14:22 +0200 (EET) Antti P Miettinen <amiettinen@nvidia.com> wrote:
> Use ARRAY_SIZE instead of sizeof to get proper max for label
> length.
>
> Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
> Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
> Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
When fixing a bug, please provide a description of the user-visible
impact of that bug. This is so that others can decide which kernel
version(s) need the patch.
Hiroshi Doyu tested this patch, so I assume there was some observable
misbehaviour to test. Please fully describe that.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: partitions: efi: Fix bound check
2013-11-15 17:14 [PATCH] fs: partitions: efi: Fix bound check Antti P Miettinen
2013-11-20 0:04 ` Andrew Morton
@ 2013-11-20 1:06 ` Davidlohr Bueso
1 sibling, 0 replies; 6+ messages in thread
From: Davidlohr Bueso @ 2013-11-20 1:06 UTC (permalink / raw)
To: amiettinen; +Cc: axboe, akpm, matt.fleming, kzak, linux-kernel
On Fri, 2013-11-15 at 19:14 +0200, Antti P Miettinen wrote:
> Use ARRAY_SIZE instead of sizeof to get proper max for label
> length.
>
> Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
> Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
> Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
I haven't tested the patch, but using ARRAY_SIZE for fields defined as
arrays does make more sense than sizeof.
Acked-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
> block/partitions/efi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index a8287b4..dc51f46 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -96,6 +96,7 @@
> * - Code works, detects all the partitions.
> *
> ************************************************************/
> +#include <linux/kernel.h>
> #include <linux/crc32.h>
> #include <linux/ctype.h>
> #include <linux/math64.h>
> @@ -715,8 +716,8 @@ int efi_partition(struct parsed_partitions *state)
> efi_guid_unparse(&ptes[i].unique_partition_guid, info->uuid);
>
> /* Naively convert UTF16-LE to 7 bits. */
> - label_max = min(sizeof(info->volname) - 1,
> - sizeof(ptes[i].partition_name));
> + label_max = min(ARRAY_SIZE(info->volname) - 1,
> + ARRAY_SIZE(ptes[i].partition_name));
> info->volname[label_max] = 0;
> while (label_count < label_max) {
> u8 c = ptes[i].partition_name[label_count] & 0xff;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: partitions: efi: Fix bound check
2013-11-20 0:04 ` Andrew Morton
@ 2013-11-20 7:18 ` Antti Miettinen
2013-11-20 7:36 ` Hiroshi Doyu
2013-11-20 17:46 ` Davidlohr Bueso
0 siblings, 2 replies; 6+ messages in thread
From: Antti Miettinen @ 2013-11-20 7:18 UTC (permalink / raw)
To: Andrew Morton
Cc: axboe@kernel.dk, davidlohr@hp.com, matt.fleming@intel.com,
kzak@redhat.com, linux-kernel@vger.kernel.org, Hiroshi Doyu
On 20.11.2013 02:04, Andrew Morton wrote:
> On Fri, 15 Nov 2013 19:14:22 +0200 (EET) Antti P Miettinen
> <amiettinen@nvidia.com> wrote:
>
> > Use ARRAY_SIZE instead of sizeof to get proper max for label
> > length.
> >
> > Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
> > Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
>
> When fixing a bug, please provide a description of the user-visible
> impact of that bug. This is so that others can decide which kernel
> version(s) need the patch.
>
> Hiroshi Doyu tested this patch, so I assume there was some observable
> misbehaviour to test. Please fully describe that.
Since this is just a read out of bounds it's not that bad, but the
problem becomes user-visible e.g. if one tries to use
CONFIG_DEBUG_PAGEALLOC and CONFIG_DEBUG_RODATA, at least with some
enhancements from Hiroshi. Of course the destination array can contain
garbage when we read beyond the end of source array so that would be
another user-visible problem.
Should I send a new version with better commit message?
--Antti
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: partitions: efi: Fix bound check
2013-11-20 7:18 ` Antti Miettinen
@ 2013-11-20 7:36 ` Hiroshi Doyu
2013-11-20 17:46 ` Davidlohr Bueso
1 sibling, 0 replies; 6+ messages in thread
From: Hiroshi Doyu @ 2013-11-20 7:36 UTC (permalink / raw)
To: akpm@linux-foundation.org, Antti Miettinen
Cc: axboe@kernel.dk, davidlohr@hp.com, matt.fleming@intel.com,
kzak@redhat.com, linux-kernel@vger.kernel.org
Antti Miettinen <amiettinen@nvidia.com> wrote @ Wed, 20 Nov 2013 08:18:50 +0100:
> On 20.11.2013 02:04, Andrew Morton wrote:
> > On Fri, 15 Nov 2013 19:14:22 +0200 (EET) Antti P Miettinen
> > <amiettinen@nvidia.com> wrote:
> >
> > > Use ARRAY_SIZE instead of sizeof to get proper max for label
> > > length.
> > >
> > > Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
> > > Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
> >
> > When fixing a bug, please provide a description of the user-visible
> > impact of that bug. This is so that others can decide which kernel
> > version(s) need the patch.
> >
> > Hiroshi Doyu tested this patch, so I assume there was some observable
> > misbehaviour to test. Please fully describe that.
>
> Since this is just a read out of bounds it's not that bad, but the
> problem becomes user-visible e.g. if one tries to use
> CONFIG_DEBUG_PAGEALLOC and CONFIG_DEBUG_RODATA, at least with some
> enhancements from Hiroshi.
The above enhancement is almost ARCH_SUPPORTS_DEBUG_PAGEALLOC for ARM,
which could catch illegal memory access(read/write) with a page fault
although that enhancement itself needs some cleanups before being
upstreamed.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fs: partitions: efi: Fix bound check
2013-11-20 7:18 ` Antti Miettinen
2013-11-20 7:36 ` Hiroshi Doyu
@ 2013-11-20 17:46 ` Davidlohr Bueso
1 sibling, 0 replies; 6+ messages in thread
From: Davidlohr Bueso @ 2013-11-20 17:46 UTC (permalink / raw)
To: Antti Miettinen
Cc: Andrew Morton, axboe@kernel.dk, matt.fleming@intel.com,
kzak@redhat.com, linux-kernel@vger.kernel.org, Hiroshi Doyu,
Will Drewry
On Wed, 2013-11-20 at 08:18 +0100, Antti Miettinen wrote:
> On 20.11.2013 02:04, Andrew Morton wrote:
> > On Fri, 15 Nov 2013 19:14:22 +0200 (EET) Antti P Miettinen
> > <amiettinen@nvidia.com> wrote:
> >
> > > Use ARRAY_SIZE instead of sizeof to get proper max for label
> > > length.
> > >
> > > Signed-off-by: Antti P Miettinen <amiettinen@nvidia.com>
> > > Reviewed-by: Hiroshi Doyu <hdoyu@nvidia.com>
> > > Tested-by: Hiroshi Doyu <hdoyu@nvidia.com>
> >
> > When fixing a bug, please provide a description of the user-visible
> > impact of that bug. This is so that others can decide which kernel
> > version(s) need the patch.
> >
> > Hiroshi Doyu tested this patch, so I assume there was some observable
> > misbehaviour to test. Please fully describe that.
>
> Since this is just a read out of bounds it's not that bad, but the
> problem becomes user-visible e.g. if one tries to use
> CONFIG_DEBUG_PAGEALLOC and CONFIG_DEBUG_RODATA, at least with some
> enhancements from Hiroshi. Of course the destination array can contain
> garbage when we read beyond the end of source array so that would be
> another user-visible problem.
>
Cc'ing Will - this path as been untouched since it's inclusion back in
2.6.37:
commit eec7ecfede74bb996060efefd5c157acd5794e8a
Author: Will Drewry <wad@chromium.org>
Date: Tue Aug 31 15:47:06 2010 -0500
genhd, efi: add efi partition metadata to hd_structs
This change extends the partition_meta_info structure to
support EFI GPT-specific metadata and ensures that data
is copied in on partition scanning.
Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
> Should I send a new version with better commit message?
>
> --Antti
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-20 17:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-15 17:14 [PATCH] fs: partitions: efi: Fix bound check Antti P Miettinen
2013-11-20 0:04 ` Andrew Morton
2013-11-20 7:18 ` Antti Miettinen
2013-11-20 7:36 ` Hiroshi Doyu
2013-11-20 17:46 ` Davidlohr Bueso
2013-11-20 1:06 ` Davidlohr Bueso
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.