grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: The development of GNU GRUB <grub-devel@gnu.org>
Subject: Re: Add check when store disk cache
Date: Wed, 14 Oct 2015 00:55:20 +0200	[thread overview]
Message-ID: <561D8BD8.50707@gmail.com> (raw)
In-Reply-To: <55FD0810.30703@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2636 bytes --]

On 19.09.2015 09:00, Andrei Borzenkov wrote:
> 18.09.2015 12:07, Arch Stack пишет:
>> I want to use the part of the filesystem codes in GRUB to read different
>> filesystems on Windows. I have almost completed it and I will release
>> it in
>> a few days.
>> But it crash sometimes because of the write of zero pointer.I debug it
>> and
>> find why it crashed.
> 
> Please show stack trace of crash. GRUB does not run multithreaded so
> something different must happen.
> 
Most likely he has just run the grub code multithreaded in his fuse-like
driver. It's probably a bad idea since GRUB is not designed for this and
in fact this lock is more for the cases when one disk code calls another
disk code. I'm not sure it actually can happen i
n current code. Moreover current way of handling ->lock isn't thread-safe
as we first check it and then set rather than using a lock primitive. If you
want to go into making parts of GRUB multi-threaded I suggest going with
giant lock
first and then breaking it down progressively. The locks should fully
disappear when compiling for real GRUB. This will probably require
preprocessor magic which makes variables disappear when GRUB_UTIL is not
defined.


>> When I apply this patch, it won't crash because of this
>> reason.
>>
>> On Fri, Sep 18, 2015 at 12:03 PM, Andrei Borzenkov <arvidjaar@gmail.com>
>> wrote:
>>
>>> 18.09.2015 03:15, Arch Stack пишет:
>>>
>>>> I found that the function *grub_disk_cache_store* didn't check for
>>>> *cache->lock* before free *cache->data*, and didn't set *cache->lock*
>>>> before memcpy something to *cache->data*. If multi thread handle
>>>> with the
>>>> same cache at the same time, it will cause a fault.
>>>>
>>>
>>> Do you actually observe a problem or it is pure hypothesis? GRUB does
>>> not
>>> run multi-threaded and probably never will.
>>>
>>> I have created a patch
>>>> for it.
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Grub-devel mailing list
>>>> Grub-devel@gnu.org
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

      reply	other threads:[~2015-10-13 22:57 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18  0:15 Add check when store disk cache Arch Stack
2015-09-18  4:03 ` Andrei Borzenkov
2015-09-18  9:07   ` Arch Stack
2015-09-19  7:00     ` Andrei Borzenkov
2015-10-13 22:55       ` Vladimir 'φ-coder/phcoder' Serbinenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=561D8BD8.50707@gmail.com \
    --to=phcoder@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).