[pve-devel] New non-thin LVM volume can contain old data

[Dominik Csapak]

On 1/22/19 9:55 AM, Herman van Rink wrote:
> Hi,
> 
> The sparse issue raised by Stoiko Ivanovin (bug 2050) made me realize
> that this could have wider security implications.
> 
> When you create a new LVM volume(not thin) you just get a range of disk
> blocks ... these are not zeroed.  So the new user of a VM gets access to
> this old 'random' data.
> 
> Steps to reproduce:
> 
> - When a disk is really empty you could first create a dummy volume and
> fill that with `yes foobar | dd of=/dev/mapper/dummylv bs=1M`, then
> lvremove it again. (The yes utility just keeps repeating the 'foobar'
> pattern.)
> 
> - Add a new disk volume to an existing vm, on a non-thin LVM storage
> - inside the vm run: `cat /dev/<new device name> | strings` # The
> strings utility skips all non-printable characters
> 
> Or to reproduce outside of Proxmox:
> 
> ```
> lvcreate --name testempty --size 1GB <vgname>
> 
> cat /dev/<vgname>/testempty | strings
> ```
> 
> In my case I got parts of my own old data. But if I we're to add a
> volume to a server where a client has access, they could get old data
> from another client.
> 
> You could argue that this is an issue in LVM, but it becomes a problem
> in a multi user system like Proxmox. Even though this may be 'by design'
> in LVM, Proxmox could help prevent users falling into this trap.
> 
> lvcreate has a --zero option, but that only clears the first 4KiB.
> 
> Another possibly related bug is 1032
> 
> Solution:
> 
> Depending on the type of storage a new volume should be filled with
> zero's before being used.
> 
> There could be a (default on) checkbox on the 'Add: Hard Disk' form to
> skip this, but can you trust all users?
> 
> The scenario I can think of where one might want to disable this
> checkbox is when your going to overwrite the whole volume anyway, and
> want to avoid the delay.
> 

you can already set the 'saferemove' property on lvm
storages, we then overwrite an lv with zeroes when one is deleted
there is also the related 'saferemove_throughput' parameter
where the throughput is limited with cstream