[pve-devel] [PATCH kernel-meta] fix #2297: trap exit to always umount ESP in pve-efiboot-tool init

[Thomas Lamprecht]

On 7/22/19 1:01 PM, Fabian Grünbichler wrote:
> On Mon, Jul 22, 2019 at 12:33:41PM +0200, Thomas Lamprecht wrote:
>> On 7/22/19 12:17 PM, Fabian Grünbichler wrote:
>>> shouldn't this be quoted like this
>>>
>>> trap '{ mountpoint -q "$esp_mp" && umount -rF "$esp_mp"; }' EXIT
>>>
>>> ? 
>>
>> No, while `shellcheck` complains the behaviour of using the value
>> $esp_mp has when setting up the trap, instead of your proposed of
>> using the value it has once the code is actually trapped, is OK here.
>> Actually both work in this case, but I explicitly put it below the
>> esp_mount= definition to ensure it's OK.
> 
> but it will still exand stuff in your version, e.g. if $UUID contains a
> special character (I am not sure whether that is possible via lsblk -
> but better safe than sorry).
> 
> e.g., $esp_mp="/var/tmp/espmounts/ABCD||echo FOO"
> 
> with your version leads to echo being executed, with mine it does not.


yes, but with both the echo will never get there in the first place ;)
The UUID comes from lsblk, and UUIDs are 128 bits rendered as hexdigits
and have at max a '-' for separation[0]..
Also, root "hacking" root was never too exciting... ;) The single real
advantage is that it cost nothing and had no disadvantage, but it adds
no safety, the lsblk output is already made for shell consumption..

[0]: effectively we get the UUID from udev and unmangle it:
https://github.com/karelzak/util-linux/blob/master/misc-utils/lsblk-properties.c#L72
https://github.com/karelzak/util-linux/blob/master/lib/mangle.c#L73
(the udev fallback to blkid is already made for shell consumption)