[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.