grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ls: prevent double open


From: Eric Snowberg
Subject: Re: [PATCH] ls: prevent double open
Date: Wed, 18 Oct 2017 15:01:20 -0600

> On Oct 18, 2017, at 11:03 AM, Vladimir 'phcoder' Serbinenko <address@hidden> 
> wrote:
> 
> 
> 
> On Wed, Oct 18, 2017, 19:02 Eric Snowberg <address@hidden> wrote:
> 
> > On Oct 18, 2017, at 10:32 AM, Vladimir 'phcoder' Serbinenko 
> > <address@hidden> wrote:
> >
> >
> >
> > On Mon, Oct 16, 2017, 22:11 Eric Snowberg <address@hidden> wrote:
> > Prevent a double open.  This can cause problems with some ieee1275
> > devices, causing the system to hang.  The double open can occur
> > as follows:
> > Why does it? Underlying firmware device should not be aware of how many 
> > times grub device is open. If it is and it causes bugs, then it's a bug in 
> > device driver
> 
> Which device driver are you referring to?  The one in GRUB or Open Firmware?
> In GRUB
> 
> 


I would agree with that.  There are many problems with the ofdisk driver in 
GRUB. One being the memory corruption issues that prevents new code from being 
added to it, which is what I believe you are recommending. I submitted a fix 
for the memory corruption problems years ago, but the patch was ignored.

memory corruption example:

devpath created in grub_ofdisk_open
  it then calls ofdisk_hash_add with devpath
    it then calls ofdisk_hash_add_real with devpath
      ofdisk_hash_add_real saves pointer of devpath
    return
  return
free devpath

dangling pointer/memory corruption with what is stored in ofdisk_hash_add_real, 
any new memory allocation corrupts devpath

As I recall, just trying to add a grub_printf in the wrong place can corrupt 
the devpath. I never understood why this bug was too risky to fix for 2.02 or 
why it still exists today. 

I ended up writing a new driver which we use instead.

https://github.com/esnowberg/grub2-sparc/commit/d802909bc614980dcf6dc2f4484bfa8c2448adf1

This driver would take care of this double open problem making this patch 
unnecessary. It will also solve many other problems that currently exist in 
ofdisk. If you would consider this driver in upstream, let me know and I’ll 
start submitting it.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]