* [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure
@ 2012-05-06 12:10 Davidlohr Bueso
2012-05-06 18:17 ` Petr Uzel
2012-05-10 9:53 ` Karel Zak
0 siblings, 2 replies; 4+ messages in thread
From: Davidlohr Bueso @ 2012-05-06 12:10 UTC (permalink / raw)
To: Karel Zak, Petr Uzel; +Cc: util-linux
From: Davidlohr Bueso <dave@gnu.org>
When the HDIO_GETGEO ioctl fails on non IRIX/MIPS platforms (ie: inappropriate ioctl for device) the variables that describe the geometry are compromissed.
One clear example is a division by 0 bug because the cylinder size is 0 is verify_sgi():
$> fdisk sgi.img
Welcome to fdisk (util-linux 2.21.392-4b1c).
...
Command (m for help): x
Expert command (m for help): g
Building a new SGI disklabel.
Partition 11 of type SGI volume and of size 7.9 MiB is set
Floating point exception
Fix this by simply exiting the program instead of leaving it in a vulnerable state.
Signed-off-by: Davidlohr Bueso <dave@gnu.org>
---
fdisk/fdisksgilabel.c | 39 ++++++++++++++++++++-------------------
1 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/fdisk/fdisksgilabel.c b/fdisk/fdisksgilabel.c
index 822f55d..b61f102 100644
--- a/fdisk/fdisksgilabel.c
+++ b/fdisk/fdisksgilabel.c
@@ -705,25 +705,26 @@ create_sgilabel(void)
res = blkdev_get_sectors(fd, &llsectors);
#ifdef HDIO_GETGEO
- if (!ioctl(fd, HDIO_GETGEO, &geometry)) {
- heads = geometry.heads;
- sectors = geometry.sectors;
- if (res == 0) {
- /* the get device size ioctl was successful */
- unsigned long long llcyls;
- llcyls = llsectors / (heads * sectors * sec_fac);
- cylinders = llcyls;
- if (cylinders != llcyls) /* truncated? */
- cylinders = ~0;
- } else {
- /* otherwise print error and use truncated version */
- cylinders = geometry.cylinders;
- fprintf(stderr,
- _("Warning: BLKGETSIZE ioctl failed on %s. "
- "Using geometry cylinder value of %d.\n"
- "This value may be truncated for devices"
- " > 33.8 GB.\n"), disk_device, cylinders);
- }
+ if (ioctl(fd, HDIO_GETGEO, &geometry) < 0)
+ err(EXIT_FAILURE, _("ioctl failed on %s"), disk_device);
+
+ heads = geometry.heads;
+ sectors = geometry.sectors;
+ if (res == 0) {
+ /* the get device size ioctl was successful */
+ unsigned long long llcyls;
+ llcyls = llsectors / (heads * sectors * sec_fac);
+ cylinders = llcyls;
+ if (cylinders != llcyls) /* truncated? */
+ cylinders = ~0;
+ } else {
+ /* otherwise print error and use truncated version */
+ cylinders = geometry.cylinders;
+ fprintf(stderr,
+ _("Warning: BLKGETSIZE ioctl failed on %s. "
+ "Using geometry cylinder value of %d.\n"
+ "This value may be truncated for devices"
+ " > 33.8 GB.\n"), disk_device, cylinders);
}
#endif
for (i = 0; i < 4; i++) {
--
1.7.4.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure
2012-05-06 12:10 [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure Davidlohr Bueso
@ 2012-05-06 18:17 ` Petr Uzel
2012-05-06 19:05 ` Davidlohr Bueso
2012-05-10 9:53 ` Karel Zak
1 sibling, 1 reply; 4+ messages in thread
From: Petr Uzel @ 2012-05-06 18:17 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: util-linux
[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]
On Sun, May 06, 2012 at 02:10:25PM +0200, Davidlohr Bueso wrote:
> From: Davidlohr Bueso <dave@gnu.org>
>
> When the HDIO_GETGEO ioctl fails on non IRIX/MIPS platforms (ie:
> inappropriate ioctl for device) the variables that describe the
> geometry are compromissed. One clear example is a division by 0 bug
> because the cylinder size is 0 is verify_sgi():
I agree with the change, however I think that more descriptive error
message should be printed; see below.
>
> $> fdisk sgi.img
> Welcome to fdisk (util-linux 2.21.392-4b1c).
>
> ...
>
> Command (m for help): x
>
> Expert command (m for help): g
> Building a new SGI disklabel.
> Partition 11 of type SGI volume and of size 7.9 MiB is set
> Floating point exception
>
> Fix this by simply exiting the program instead of leaving it in a vulnerable state.
>
> Signed-off-by: Davidlohr Bueso <dave@gnu.org>
> ---
> fdisk/fdisksgilabel.c | 39 ++++++++++++++++++++-------------------
> 1 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/fdisk/fdisksgilabel.c b/fdisk/fdisksgilabel.c
> index 822f55d..b61f102 100644
> --- a/fdisk/fdisksgilabel.c
> +++ b/fdisk/fdisksgilabel.c
> @@ -705,25 +705,26 @@ create_sgilabel(void)
> res = blkdev_get_sectors(fd, &llsectors);
>
> #ifdef HDIO_GETGEO
> - if (!ioctl(fd, HDIO_GETGEO, &geometry)) {
> - heads = geometry.heads;
> - sectors = geometry.sectors;
> - if (res == 0) {
> - /* the get device size ioctl was successful */
> - unsigned long long llcyls;
> - llcyls = llsectors / (heads * sectors * sec_fac);
> - cylinders = llcyls;
> - if (cylinders != llcyls) /* truncated? */
> - cylinders = ~0;
> - } else {
> - /* otherwise print error and use truncated version */
> - cylinders = geometry.cylinders;
> - fprintf(stderr,
> - _("Warning: BLKGETSIZE ioctl failed on %s. "
> - "Using geometry cylinder value of %d.\n"
> - "This value may be truncated for devices"
> - " > 33.8 GB.\n"), disk_device, cylinders);
> - }
> + if (ioctl(fd, HDIO_GETGEO, &geometry) < 0)
> + err(EXIT_FAILURE, _("ioctl failed on %s"), disk_device);
s/"ioctl failed..."/"HDIO_GETGEO ioctl failed..."
[SNIP]
Petr
--
Petr Uzel
IRC: ptr_uzl @ freenode
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure
2012-05-06 18:17 ` Petr Uzel
@ 2012-05-06 19:05 ` Davidlohr Bueso
0 siblings, 0 replies; 4+ messages in thread
From: Davidlohr Bueso @ 2012-05-06 19:05 UTC (permalink / raw)
To: Petr Uzel; +Cc: util-linux
On Sun, 2012-05-06 at 20:17 +0200, Petr Uzel wrote:
> On Sun, May 06, 2012 at 02:10:25PM +0200, Davidlohr Bueso wrote:
> > From: Davidlohr Bueso <dave@gnu.org>
> >
> > When the HDIO_GETGEO ioctl fails on non IRIX/MIPS platforms (ie:
> > inappropriate ioctl for device) the variables that describe the
> > geometry are compromissed. One clear example is a division by 0 bug
> > because the cylinder size is 0 is verify_sgi():
>
> I agree with the change, however I think that more descriptive error
> message should be printed; see below.
>
> >
> > $> fdisk sgi.img
> > Welcome to fdisk (util-linux 2.21.392-4b1c).
> >
> > ...
> >
> > Command (m for help): x
> >
> > Expert command (m for help): g
> > Building a new SGI disklabel.
> > Partition 11 of type SGI volume and of size 7.9 MiB is set
> > Floating point exception
> >
> > Fix this by simply exiting the program instead of leaving it in a vulnerable state.
> >
> > Signed-off-by: Davidlohr Bueso <dave@gnu.org>
> > ---
> > fdisk/fdisksgilabel.c | 39 ++++++++++++++++++++-------------------
> > 1 files changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/fdisk/fdisksgilabel.c b/fdisk/fdisksgilabel.c
> > index 822f55d..b61f102 100644
> > --- a/fdisk/fdisksgilabel.c
> > +++ b/fdisk/fdisksgilabel.c
> > @@ -705,25 +705,26 @@ create_sgilabel(void)
> > res = blkdev_get_sectors(fd, &llsectors);
> >
> > #ifdef HDIO_GETGEO
> > - if (!ioctl(fd, HDIO_GETGEO, &geometry)) {
> > - heads = geometry.heads;
> > - sectors = geometry.sectors;
> > - if (res == 0) {
> > - /* the get device size ioctl was successful */
> > - unsigned long long llcyls;
> > - llcyls = llsectors / (heads * sectors * sec_fac);
> > - cylinders = llcyls;
> > - if (cylinders != llcyls) /* truncated? */
> > - cylinders = ~0;
> > - } else {
> > - /* otherwise print error and use truncated version */
> > - cylinders = geometry.cylinders;
> > - fprintf(stderr,
> > - _("Warning: BLKGETSIZE ioctl failed on %s. "
> > - "Using geometry cylinder value of %d.\n"
> > - "This value may be truncated for devices"
> > - " > 33.8 GB.\n"), disk_device, cylinders);
> > - }
> > + if (ioctl(fd, HDIO_GETGEO, &geometry) < 0)
> > + err(EXIT_FAILURE, _("ioctl failed on %s"), disk_device);
>
> s/"ioctl failed..."/"HDIO_GETGEO ioctl failed..."
I don't care either way - perhaps even a "cannot get geometry" message.
Karel, its up to you.
Thanks for looking into this patchet!
- Davidlohr
>
> [SNIP]
>
> Petr
>
> --
> Petr Uzel
> IRC: ptr_uzl @ freenode
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure
2012-05-06 12:10 [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure Davidlohr Bueso
2012-05-06 18:17 ` Petr Uzel
@ 2012-05-10 9:53 ` Karel Zak
1 sibling, 0 replies; 4+ messages in thread
From: Karel Zak @ 2012-05-10 9:53 UTC (permalink / raw)
To: Davidlohr Bueso; +Cc: Petr Uzel, util-linux
On Sun, May 06, 2012 at 02:10:25PM +0200, Davidlohr Bueso wrote:
> fdisk/fdisksgilabel.c | 39 ++++++++++++++++++++-------------------
> 1 files changed, 20 insertions(+), 19 deletions(-)
Applied, thanks.
--
Karel Zak <kzak@redhat.com>
http://karelzak.blogspot.com
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-05-10 9:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-06 12:10 [PATCH 5/5] fdisk: sgi: abort on HDIO_GETGEO failure Davidlohr Bueso
2012-05-06 18:17 ` Petr Uzel
2012-05-06 19:05 ` Davidlohr Bueso
2012-05-10 9:53 ` Karel Zak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox