Re: Add check when store disk cache

[Vladimir 'φ-coder/phcoder' Serbinenko]

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 <address@hidden>
>> 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
>>>> address@hidden
>>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> address@hidden
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel

signature.asc
Description: OpenPGP digital signature