Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

efivars: efivarfs_valid_name() should handle pstore syntax

Stricter validation was introduced with commit da27a24383b2b
("efivarfs: guid part of filenames are case-insensitive") and commit
47f531e8ba3b ("efivarfs: Validate filenames much more aggressively"),
which is necessary for the guid portion of efivarfs filenames, but we
don't need to be so strict with the first part, the variable name. The
UEFI specification doesn't impose any constraints on variable names
other than they be a NULL-terminated string.

The above commits caused a regression that resulted in users seeing
the following message,

$ sudo mount -v /sys/firmware/efi/efivars mount: Cannot allocate memory

whenever pstore EFI variables were present in the variable store,
since their variable names failed to pass the following check,

/* GUID should be right after the first '-' */
if (s - 1 != strchr(str, '-'))

as a typical pstore filename is of the form, dump-type0-10-1-<guid>.
The fix is trivial since the guid portion of the filename is GUID_LEN
bytes, we can use (len - GUID_LEN) to ensure the '-' character is
where we expect it to be.

(The bogus ENOMEM error value will be fixed in a separate patch.)

Reported-by: Joseph Yasi <joe.yasi@gmail.com>
Tested-by: Joseph Yasi <joe.yasi@gmail.com>
Reported-by: Lingzhu Xiang <lxiang@redhat.com>
Cc: Josh Boyer <jwboyer@redhat.com>
Cc: Jeremy Kerr <jk@ozlabs.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: <stable@vger.kernel.org> # v3.8
Signed-off-by: Matt Fleming <matt.fleming@intel.com>

+61 -2
+2 -2
drivers/firmware/efivars.c
··· 974 974 if (len < GUID_LEN + 2) 975 975 return false; 976 976 977 - /* GUID should be right after the first '-' */ 978 - if (s - 1 != strchr(str, '-')) 977 + /* GUID must be preceded by a '-' */ 978 + if (*(s - 1) != '-') 979 979 return false; 980 980 981 981 /*
+59
tools/testing/selftests/efivarfs/efivarfs.sh
··· 125 125 ./open-unlink $file 126 126 } 127 127 128 + # test that we can create a range of filenames 129 + test_valid_filenames() 130 + { 131 + local attrs='\x07\x00\x00\x00' 132 + local ret=0 133 + 134 + local file_list="abc dump-type0-11-1-1362436005 1234 -" 135 + for f in $file_list; do 136 + local file=$efivarfs_mount/$f-$test_guid 137 + 138 + printf "$attrs\x00" > $file 139 + 140 + if [ ! -e $file ]; then 141 + echo "$file could not be created" >&2 142 + ret=1 143 + else 144 + rm $file 145 + fi 146 + done 147 + 148 + exit $ret 149 + } 150 + 151 + test_invalid_filenames() 152 + { 153 + local attrs='\x07\x00\x00\x00' 154 + local ret=0 155 + 156 + local file_list=" 157 + -1234-1234-1234-123456789abc 158 + foo 159 + foo-bar 160 + -foo- 161 + foo-barbazba-foob-foob-foob-foobarbazfoo 162 + foo------------------------------------- 163 + -12345678-1234-1234-1234-123456789abc 164 + a-12345678=1234-1234-1234-123456789abc 165 + a-12345678-1234=1234-1234-123456789abc 166 + a-12345678-1234-1234=1234-123456789abc 167 + a-12345678-1234-1234-1234=123456789abc 168 + 1112345678-1234-1234-1234-123456789abc" 169 + 170 + for f in $file_list; do 171 + local file=$efivarfs_mount/$f 172 + 173 + printf "$attrs\x00" 2>/dev/null > $file 174 + 175 + if [ -e $file ]; then 176 + echo "Creating $file should have failed" >&2 177 + rm $file 178 + ret=1 179 + fi 180 + done 181 + 182 + exit $ret 183 + } 184 + 128 185 check_prereqs 129 186 130 187 rc=0 ··· 192 135 run_test test_delete 193 136 run_test test_zero_size_delete 194 137 run_test test_open_unlink 138 + run_test test_valid_filenames 139 + run_test test_invalid_filenames 195 140 196 141 exit $rc