efivars write(2) races
1) process A does write() on efivars file, reaches ->get_variable(),
gets newdatasize set, drops efivars->lock and loses CPU before an attempt to grab ->i_mutex. process B comes and does the same thing, replacing the variable contents. Then it grabs ->i_mutex, updates size, drops ->i_mutex and buggers off. At which point A gets CPU back and proceeds to set size to whatever would be valid for its write. Only the value is bogus now...
2) what's to prevent EFI_NOT_FOUND being hit twice? Bad things
will obviously happen in that case...
Quoted 6 lines · Al Viro
1) process A does
write()on efivars file, reaches ->get_variable(),
gets newdatasize set, dropsefivars->lockand loses CPU before an attempt to grab ->i_mutex. process B comes and does the same thing, replacing the variable contents. Then it grabs ->i_mutex, updates size, drops ->i_mutex and buggers off. At which point A gets CPU back and proceeds to set size to whatever would be valid for its write. Only the value is bogus now...
There are a few other things that makes size bogus now.
1. truncate() never touches nvram but pretends to be changing size.
2. Empty files come back with non-zero size after remount. They are imported from sysfs when mounting.
3. Arguably reading empty files could just return empty instead of returning EIO/EFI_NOT_FOUND from firmware.
4. EFI_VARIABLE_APPEND_WRITE with EFI_OUT_OF_RESOURCES truncates size but you can still read its content.
Quoted 11 lines · Lingzhu Xiang
Quoted 6 lines · Al Viro
1) process A does
write()on efivars file, reaches ->get_variable(),
gets newdatasize set, dropsefivars->lockand loses CPU before an attempt to grab ->i_mutex. process B comes and does the same thing, replacing the variable contents. Then it grabs ->i_mutex, updates size, drops ->i_mutex and buggers off. At which point A gets CPU back and proceeds to set size to whatever would be valid for its write. Only the value is bogus now...There are a few other things that makes size bogus now.
1.
truncate()never touches nvram but pretends to be changing size.
Good catch. Looks like we need to provide an implementation for this.
Quoted 2 lines
2. Empty files come back with non-zero size after remount. They are imported from sysfs when mounting.
Eeek. The return value of get_variable() isn't checked in efivarfs_fill_super().
Quoted 2 lines
3. Arguably reading empty files could just return empty instead of returning EIO/EFI_NOT_FOUND from firmware.
Yeah, I think Jeremy has a patch to fix this.
Quoted 2 lines
4.
EFI_VARIABLE_APPEND_WRITEwithEFI_OUT_OF_RESOURCEStruncates size but you can still read its content.
I'm not sure what you mean by this. Could you please explain?
Thanks for the feedback.
Quoted 4 lines · Matt Fleming
Quoted 2 lines
4.
EFI_VARIABLE_APPEND_WRITEwithEFI_OUT_OF_RESOURCEStruncates size but you can still read its content.I'm not sure what you mean by this. Could you please explain?
Oops, this one is incorrect. I was testing EFI_VARIABLE_APPEND_WRITE using this:
printf "\x47\x00\x00\x00\x00" >test-1-$guid
So it truncates the file before writing, but efivarfs figures out the right size afterwards. The size remains zero if the write fails.
I should be using this for appending:
printf "\x47\x00\x00\x00\x00" >>test-1-$guid
Quoted 9 lines · Lingzhu Xiang
Quoted 4 lines · Matt Fleming
Quoted 2 lines
4.
EFI_VARIABLE_APPEND_WRITEwithEFI_OUT_OF_RESOURCEStruncates size but you can still read its content.I'm not sure what you mean by this. Could you please explain?
Oops, this one is incorrect. I was testing
EFI_VARIABLE_APPEND_WRITEusing this:
Quoted 8 lines
printf "\x47\x00\x00\x00\x00" >test-1-$guid
So it truncates the file before writing, but efivarfs figures out the right size afterwards. The size remains zero if the write fails.
I should be using this for appending:
printf "\x47\x00\x00\x00\x00" >>test-1-$guid
Does this mean that these two test cases work as expected?
Quoted 12 lines · Matt Fleming
Quoted 11 lines · Lingzhu Xiang
Quoted 4 lines · Matt Fleming
Quoted 2 lines
4.
EFI_VARIABLE_APPEND_WRITEwithEFI_OUT_OF_RESOURCEStruncates size but you can still read its content.I'm not sure what you mean by this. Could you please explain?
Oops, this one is incorrect. I was testing
EFI_VARIABLE_APPEND_WRITEusing this:printf "\x47\x00\x00\x00\x00" >test-1-$guid
Quoted 8 lines
Quoted 6 lines
So it truncates the file before writing, but efivarfs figures out the right size afterwards. The size remains zero if the write fails.
I should be using this for appending:
printf "\x47\x00\x00\x00\x00" >>test-1-$guid
Does this mean that these two test cases work as expected?
Mostly yes.
printf "\x47\x00\x00\x00\x00" >test-1-$guid
still shows unexpected truncate behavior. But this is the same one already noted previously (truncate doesn't actually work).
Quoted 6 lines · Al Viro
1) process A does
write()on efivars file, reaches ->get_variable(),
gets newdatasize set, dropsefivars->lockand loses CPU before an attempt to grab ->i_mutex. process B comes and does the same thing, replacing the variable contents. Then it grabs ->i_mutex, updates size, drops ->i_mutex and buggers off. At which point A gets CPU back and proceeds to set size to whatever would be valid for its write. Only the value is bogus now...
Crap, yes. We'll have to jiggle the locking rules around to fix this one.
Quoted 2 lines
2) what's to prevent
EFI_NOT_FOUNDbeing hit twice? Bad things
will obviously happen in that case...
I'm not sure which execution path will trigger this? Oh, if we're spinning for the spinlock in efivarfs_unlink() while another process does a delete via efivarfs_file_write()? Yeah, that's gonna cause some tears.
Thanks, Al.