[SLOF] [PATCH 0/4] Block write support for SCSI and virtio-block disks

[Thomas Huth]

On 15.11.2016 03:54, Nikunj A Dadhania wrote:
> Thomas Huth <thuth at redhat.com> writes:
> 
>> On 14.11.2016 08:53, Nikunj A Dadhania wrote:
>>> Thomas Huth <thuth at redhat.com> writes:
>>>
>>>> On 14.11.2016 07:32, Nikunj A Dadhania wrote:
>> [...]
>>>>> My only worry here is that it would open up a way to write to the
>>>>> critical section of the disk image from the SLOF prompt. Is there a way
>>>>> we can prevent this?
>>>>
>>>> Good idea, I also felt a little bit uneasy to have write support in the
>>>> firmware, but since GRUB needs it, we likely can't ignore this.
>>>> So with critical section, you mean the MBR, I assume?
>>>
>>> Yes.
>>>
>>>> That should be feasible, I think I could add a check that refuses
>>>> writes to the first 512 bytes (or a little bit more to also protect
>>>> the GPT? Suggestions welcome!).
>>>
>>> Correct. For MBR 1st sector. For GPT (34 sectors in the beginning and 33
>>> at the end) please refer to the following link for more details
>>>
>>> https://en.wikipedia.org/wiki/GUID_Partition_Table
>>
>> I just had a look at it, but adding code for checking whether the GPT is
>> available or not (or using the checks from disk-label.fs) would render
>> the whole checking mechanism quite complicated, as far as I can see...
> 
> : write 
> \ IF sector number is 0 return
> 
> no-gpt? ! IF
>     \ check-block number
> THEN
> 
> \ call write
> ;
> 
> Shouldn't something like the above work fine? Am I missing something?

The problem is: The GPT stuff is handled at the disk-label.fs level, the
sector write stuff is done at the driver / deblocker level instead.
Let me try to paint an ASCII art picture:


           +------------+
           |            |
ihandle ---> disk-label |
           |            |
           +------------+
                 |
      read/write |
                 |
           +-----v------+   read/write   +-----------+
           |            +---------------->           |
           |   driver   |                | deblocker |
           | (SCSI...)  <----------------+           |
           +------------+   read-blocks  +-----------+
                 |          write-blocks
                 v
            lower layers

The current offset is stored in the deblocker, the GPT information in
the disk-label. Without adding some ugly hacks, there is currently now
way to combine the offset information with the GPT information to do
this check properly, as far as I can see.

>> What about simply refusing write accesses to the first 4 sectors or so?
>> Would that be OK?
> 
> That would not cover all GPT sectors

OK. What about blocking writes at the driver level to the first 34
sectors? According to the
https://en.wikipedia.org/wiki/GUID_Partition_Table article, even with
old MBR based layouts, the first partition normally never starts before
LBA sector 63 (since it should be aligned on "cylinders"), so we should
never end up writing to the first 34 sectors that could contain the GPT.

Additionally I'll implement blocking of write access to the whole disk
at the disk-label level, so you can only write if you opened the device
tree node with a partition number specified. I think that should give us
enough safety here.

 Thomas