Commit a3c70636 authored by Peter Jones's avatar Peter Jones Committed by Luis Henriques

efi: Make efivarfs entries immutable by default

commit ed8b0de5 upstream.

"rm -rf" is bricking some peoples' laptops because of variables being
used to store non-reinitializable firmware driver data that's required
to POST the hardware.

These are 100% bugs, and they need to be fixed, but in the mean time it
shouldn't be easy to *accidentally* brick machines.

We have to have delete working, and picking which variables do and don't
work for deletion is quite intractable, so instead make everything
immutable by default (except for a whitelist), and make tools that
aren't quite so broad-spectrum unset the immutable flag.
Signed-off-by: default avatarPeter Jones <pjones@redhat.com>
Tested-by: default avatarLee, Chun-Yi <jlee@suse.com>
Acked-by: default avatarMatthew Garrett <mjg59@coreos.com>
Signed-off-by: default avatarMatt Fleming <matt@codeblueprint.co.uk>
[ luis: backported to 3.16:
  - use mutex_lock/unlock() instead of inode_lock/unlock()
  - use root->d_inode instead of d_inode()  ]
Signed-off-by: default avatarLuis Henriques <luis.henriques@canonical.com>
parent 67217cf5
...@@ -14,3 +14,10 @@ filesystem. ...@@ -14,3 +14,10 @@ filesystem.
efivarfs is typically mounted like this, efivarfs is typically mounted like this,
mount -t efivarfs none /sys/firmware/efi/efivars mount -t efivarfs none /sys/firmware/efi/efivars
Due to the presence of numerous firmware bugs where removing non-standard
UEFI variables causes the system firmware to fail to POST, efivarfs
files that are not well-known standardized variables are created
as immutable files. This doesn't prevent removal - "chattr -i" will work -
but it does prevent this kind of failure from being accomplished
accidentally.
...@@ -172,10 +172,12 @@ struct variable_validate { ...@@ -172,10 +172,12 @@ struct variable_validate {
}; };
/* /*
* This is the list of variables we need to validate. * This is the list of variables we need to validate, as well as the
* whitelist for what we think is safe not to default to immutable.
* *
* If it has a validate() method that's not NULL, it'll go into the * If it has a validate() method that's not NULL, it'll go into the
* validation routine. If not, it is assumed valid. * validation routine. If not, it is assumed valid, but still used for
* whitelisting.
* *
* Note that it's sorted by {vendor,name}, but globbed names must come after * Note that it's sorted by {vendor,name}, but globbed names must come after
* any other name with the same prefix. * any other name with the same prefix.
...@@ -193,11 +195,37 @@ static const struct variable_validate variable_validate[] = { ...@@ -193,11 +195,37 @@ static const struct variable_validate variable_validate[] = {
{ EFI_GLOBAL_VARIABLE_GUID, "ErrOut", validate_device_path }, { EFI_GLOBAL_VARIABLE_GUID, "ErrOut", validate_device_path },
{ EFI_GLOBAL_VARIABLE_GUID, "ErrOutDev", validate_device_path }, { EFI_GLOBAL_VARIABLE_GUID, "ErrOutDev", validate_device_path },
{ EFI_GLOBAL_VARIABLE_GUID, "Lang", validate_ascii_string }, { EFI_GLOBAL_VARIABLE_GUID, "Lang", validate_ascii_string },
{ EFI_GLOBAL_VARIABLE_GUID, "OsIndications", NULL },
{ EFI_GLOBAL_VARIABLE_GUID, "PlatformLang", validate_ascii_string }, { EFI_GLOBAL_VARIABLE_GUID, "PlatformLang", validate_ascii_string },
{ EFI_GLOBAL_VARIABLE_GUID, "Timeout", validate_uint16 }, { EFI_GLOBAL_VARIABLE_GUID, "Timeout", validate_uint16 },
{ NULL_GUID, "", NULL }, { NULL_GUID, "", NULL },
}; };
static bool
variable_matches(const char *var_name, size_t len, const char *match_name,
int *match)
{
for (*match = 0; ; (*match)++) {
char c = match_name[*match];
char u = var_name[*match];
/* Wildcard in the matching name means we've matched */
if (c == '*')
return true;
/* Case sensitive match */
if (!c && *match == len)
return true;
if (c != u)
return false;
if (!c)
return true;
}
return true;
}
bool bool
efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data, efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
unsigned long data_size) unsigned long data_size)
...@@ -221,35 +249,48 @@ efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data, ...@@ -221,35 +249,48 @@ efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
if (efi_guidcmp(vendor, variable_validate[i].vendor)) if (efi_guidcmp(vendor, variable_validate[i].vendor))
continue; continue;
for (match = 0; ; match++) { if (variable_matches(utf8_name, utf8_size+1, name, &match)) {
char c = name[match]; if (variable_validate[i].validate == NULL)
char u = utf8_name[match];
/* Wildcard in the matching name means we've matched */
if (c == '*') {
kfree(utf8_name);
return variable_validate[i].validate(var_name,
match, data, data_size);
}
/* Case sensitive match */
if (c != u)
break; break;
kfree(utf8_name);
/* Reached the end of the string while matching */ return variable_validate[i].validate(var_name, match,
if (!c) { data, data_size);
kfree(utf8_name);
return variable_validate[i].validate(var_name,
match, data, data_size);
}
} }
} }
kfree(utf8_name); kfree(utf8_name);
return true; return true;
} }
EXPORT_SYMBOL_GPL(efivar_validate); EXPORT_SYMBOL_GPL(efivar_validate);
bool
efivar_variable_is_removable(efi_guid_t vendor, const char *var_name,
size_t len)
{
int i;
bool found = false;
int match = 0;
/*
* Check if our variable is in the validated variables list
*/
for (i = 0; variable_validate[i].name[0] != '\0'; i++) {
if (efi_guidcmp(variable_validate[i].vendor, vendor))
continue;
if (variable_matches(var_name, len,
variable_validate[i].name, &match)) {
found = true;
break;
}
}
/*
* If it's in our list, it is removable.
*/
return found;
}
EXPORT_SYMBOL_GPL(efivar_variable_is_removable);
static efi_status_t static efi_status_t
check_var_size(u32 attributes, unsigned long size) check_var_size(u32 attributes, unsigned long size)
{ {
......
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <linux/efi.h> #include <linux/efi.h>
#include <linux/fs.h> #include <linux/fs.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/mount.h>
#include "internal.h" #include "internal.h"
...@@ -103,9 +104,78 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf, ...@@ -103,9 +104,78 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
return size; return size;
} }
static int
efivarfs_ioc_getxflags(struct file *file, void __user *arg)
{
struct inode *inode = file->f_mapping->host;
unsigned int i_flags;
unsigned int flags = 0;
i_flags = inode->i_flags;
if (i_flags & S_IMMUTABLE)
flags |= FS_IMMUTABLE_FL;
if (copy_to_user(arg, &flags, sizeof(flags)))
return -EFAULT;
return 0;
}
static int
efivarfs_ioc_setxflags(struct file *file, void __user *arg)
{
struct inode *inode = file->f_mapping->host;
unsigned int flags;
unsigned int i_flags = 0;
int error;
if (!inode_owner_or_capable(inode))
return -EACCES;
if (copy_from_user(&flags, arg, sizeof(flags)))
return -EFAULT;
if (flags & ~FS_IMMUTABLE_FL)
return -EOPNOTSUPP;
if (!capable(CAP_LINUX_IMMUTABLE))
return -EPERM;
if (flags & FS_IMMUTABLE_FL)
i_flags |= S_IMMUTABLE;
error = mnt_want_write_file(file);
if (error)
return error;
mutex_lock(&inode->i_mutex);
inode_set_flags(inode, i_flags, S_IMMUTABLE);
mutex_unlock(&inode->i_mutex);
mnt_drop_write_file(file);
return 0;
}
long
efivarfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long p)
{
void __user *arg = (void __user *)p;
switch (cmd) {
case FS_IOC_GETFLAGS:
return efivarfs_ioc_getxflags(file, arg);
case FS_IOC_SETFLAGS:
return efivarfs_ioc_setxflags(file, arg);
}
return -ENOTTY;
}
const struct file_operations efivarfs_file_operations = { const struct file_operations efivarfs_file_operations = {
.open = simple_open, .open = simple_open,
.read = efivarfs_file_read, .read = efivarfs_file_read,
.write = efivarfs_file_write, .write = efivarfs_file_write,
.llseek = no_llseek, .llseek = no_llseek,
.unlocked_ioctl = efivarfs_file_ioctl,
}; };
...@@ -15,7 +15,8 @@ ...@@ -15,7 +15,8 @@
#include "internal.h" #include "internal.h"
struct inode *efivarfs_get_inode(struct super_block *sb, struct inode *efivarfs_get_inode(struct super_block *sb,
const struct inode *dir, int mode, dev_t dev) const struct inode *dir, int mode,
dev_t dev, bool is_removable)
{ {
struct inode *inode = new_inode(sb); struct inode *inode = new_inode(sb);
...@@ -23,6 +24,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb, ...@@ -23,6 +24,7 @@ struct inode *efivarfs_get_inode(struct super_block *sb,
inode->i_ino = get_next_ino(); inode->i_ino = get_next_ino();
inode->i_mode = mode; inode->i_mode = mode;
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
inode->i_flags = is_removable ? 0 : S_IMMUTABLE;
switch (mode & S_IFMT) { switch (mode & S_IFMT) {
case S_IFREG: case S_IFREG:
inode->i_fop = &efivarfs_file_operations; inode->i_fop = &efivarfs_file_operations;
...@@ -102,22 +104,17 @@ static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid) ...@@ -102,22 +104,17 @@ static void efivarfs_hex_to_guid(const char *str, efi_guid_t *guid)
static int efivarfs_create(struct inode *dir, struct dentry *dentry, static int efivarfs_create(struct inode *dir, struct dentry *dentry,
umode_t mode, bool excl) umode_t mode, bool excl)
{ {
struct inode *inode; struct inode *inode = NULL;
struct efivar_entry *var; struct efivar_entry *var;
int namelen, i = 0, err = 0; int namelen, i = 0, err = 0;
bool is_removable = false;
if (!efivarfs_valid_name(dentry->d_name.name, dentry->d_name.len)) if (!efivarfs_valid_name(dentry->d_name.name, dentry->d_name.len))
return -EINVAL; return -EINVAL;
inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0);
if (!inode)
return -ENOMEM;
var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL); var = kzalloc(sizeof(struct efivar_entry), GFP_KERNEL);
if (!var) { if (!var)
err = -ENOMEM; return -ENOMEM;
goto out;
}
/* length of the variable name itself: remove GUID and separator */ /* length of the variable name itself: remove GUID and separator */
namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1; namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1;
...@@ -125,6 +122,16 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, ...@@ -125,6 +122,16 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1, efivarfs_hex_to_guid(dentry->d_name.name + namelen + 1,
&var->var.VendorGuid); &var->var.VendorGuid);
if (efivar_variable_is_removable(var->var.VendorGuid,
dentry->d_name.name, namelen))
is_removable = true;
inode = efivarfs_get_inode(dir->i_sb, dir, mode, 0, is_removable);
if (!inode) {
err = -ENOMEM;
goto out;
}
for (i = 0; i < namelen; i++) for (i = 0; i < namelen; i++)
var->var.VariableName[i] = dentry->d_name.name[i]; var->var.VariableName[i] = dentry->d_name.name[i];
...@@ -138,7 +145,8 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, ...@@ -138,7 +145,8 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry,
out: out:
if (err) { if (err) {
kfree(var); kfree(var);
iput(inode); if (inode)
iput(inode);
} }
return err; return err;
} }
......
...@@ -15,7 +15,8 @@ extern const struct file_operations efivarfs_file_operations; ...@@ -15,7 +15,8 @@ extern const struct file_operations efivarfs_file_operations;
extern const struct inode_operations efivarfs_dir_inode_operations; extern const struct inode_operations efivarfs_dir_inode_operations;
extern bool efivarfs_valid_name(const char *str, int len); extern bool efivarfs_valid_name(const char *str, int len);
extern struct inode *efivarfs_get_inode(struct super_block *sb, extern struct inode *efivarfs_get_inode(struct super_block *sb,
const struct inode *dir, int mode, dev_t dev); const struct inode *dir, int mode, dev_t dev,
bool is_removable);
extern struct list_head efivarfs_list; extern struct list_head efivarfs_list;
......
...@@ -120,6 +120,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor, ...@@ -120,6 +120,7 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
char *name; char *name;
int len; int len;
int err = -ENOMEM; int err = -ENOMEM;
bool is_removable = false;
entry = kzalloc(sizeof(*entry), GFP_KERNEL); entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry) if (!entry)
...@@ -137,13 +138,17 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor, ...@@ -137,13 +138,17 @@ static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
ucs2_as_utf8(name, entry->var.VariableName, len); ucs2_as_utf8(name, entry->var.VariableName, len);
if (efivar_variable_is_removable(entry->var.VendorGuid, name, len))
is_removable = true;
name[len] = '-'; name[len] = '-';
efi_guid_unparse(&entry->var.VendorGuid, name + len + 1); efi_guid_unparse(&entry->var.VendorGuid, name + len + 1);
name[len + EFI_VARIABLE_GUID_LEN+1] = '\0'; name[len + EFI_VARIABLE_GUID_LEN+1] = '\0';
inode = efivarfs_get_inode(sb, root->d_inode, S_IFREG | 0644, 0); inode = efivarfs_get_inode(sb, root->d_inode, S_IFREG | 0644, 0,
is_removable);
if (!inode) if (!inode)
goto fail_name; goto fail_name;
...@@ -199,7 +204,7 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent) ...@@ -199,7 +204,7 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
sb->s_d_op = &efivarfs_d_ops; sb->s_d_op = &efivarfs_d_ops;
sb->s_time_gran = 1; sb->s_time_gran = 1;
inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0); inode = efivarfs_get_inode(sb, NULL, S_IFDIR | 0755, 0, true);
if (!inode) if (!inode)
return -ENOMEM; return -ENOMEM;
inode->i_op = &efivarfs_dir_inode_operations; inode->i_op = &efivarfs_dir_inode_operations;
......
...@@ -1145,6 +1145,8 @@ struct efivar_entry *efivar_entry_find(efi_char16_t *name, efi_guid_t guid, ...@@ -1145,6 +1145,8 @@ struct efivar_entry *efivar_entry_find(efi_char16_t *name, efi_guid_t guid,
bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data, bool efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
unsigned long data_size); unsigned long data_size);
bool efivar_variable_is_removable(efi_guid_t vendor, const char *name,
size_t len);
extern struct work_struct efivar_work; extern struct work_struct efivar_work;
void efivar_run_worker(void); void efivar_run_worker(void);
......
...@@ -88,7 +88,11 @@ test_delete() ...@@ -88,7 +88,11 @@ test_delete()
exit 1 exit 1
fi fi
rm $file rm $file 2>/dev/null
if [ $? -ne 0 ]; then
chattr -i $file
rm $file
fi
if [ -e $file ]; then if [ -e $file ]; then
echo "$file couldn't be deleted" >&2 echo "$file couldn't be deleted" >&2
...@@ -111,6 +115,7 @@ test_zero_size_delete() ...@@ -111,6 +115,7 @@ test_zero_size_delete()
exit 1 exit 1
fi fi
chattr -i $file
printf "$attrs" > $file printf "$attrs" > $file
if [ -e $file ]; then if [ -e $file ]; then
...@@ -141,7 +146,11 @@ test_valid_filenames() ...@@ -141,7 +146,11 @@ test_valid_filenames()
echo "$file could not be created" >&2 echo "$file could not be created" >&2
ret=1 ret=1
else else
rm $file rm $file 2>/dev/null
if [ $? -ne 0 ]; then
chattr -i $file
rm $file
fi
fi fi
done done
...@@ -174,7 +183,11 @@ test_invalid_filenames() ...@@ -174,7 +183,11 @@ test_invalid_filenames()
if [ -e $file ]; then if [ -e $file ]; then
echo "Creating $file should have failed" >&2 echo "Creating $file should have failed" >&2
rm $file rm $file 2>/dev/null
if [ $? -ne 0 ]; then
chattr -i $file
rm $file
fi
ret=1 ret=1
fi fi
done done
......
#include <errno.h>
#include <stdio.h> #include <stdio.h>
#include <stdint.h> #include <stdint.h>
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h> #include <unistd.h>
#include <sys/ioctl.h>
#include <sys/types.h> #include <sys/types.h>
#include <sys/stat.h> #include <sys/stat.h>
#include <fcntl.h> #include <fcntl.h>
#include <linux/fs.h>
static int set_immutable(const char *path, int immutable)
{
unsigned int flags;
int fd;
int rc;
int error;
fd = open(path, O_RDONLY);
if (fd < 0)
return fd;
rc = ioctl(fd, FS_IOC_GETFLAGS, &flags);
if (rc < 0) {
error = errno;
close(fd);
errno = error;
return rc;
}
if (immutable)
flags |= FS_IMMUTABLE_FL;
else
flags &= ~FS_IMMUTABLE_FL;
rc = ioctl(fd, FS_IOC_SETFLAGS, &flags);
error = errno;
close(fd);
errno = error;
return rc;
}
static int get_immutable(const char *path)
{
unsigned int flags;
int fd;
int rc;
int error;
fd = open(path, O_RDONLY);
if (fd < 0)
return fd;
rc = ioctl(fd, FS_IOC_GETFLAGS, &flags);
if (rc < 0) {
error = errno;
close(fd);
errno = error;
return rc;
}
close(fd);
if (flags & FS_IMMUTABLE_FL)
return 1;
return 0;
}
int main(int argc, char **argv) int main(int argc, char **argv)
{ {
...@@ -27,7 +85,7 @@ int main(int argc, char **argv) ...@@ -27,7 +85,7 @@ int main(int argc, char **argv)
buf[4] = 0; buf[4] = 0;
/* create a test variable */ /* create a test variable */
fd = open(path, O_WRONLY | O_CREAT); fd = open(path, O_WRONLY | O_CREAT, 0600);
if (fd < 0) { if (fd < 0) {
perror("open(O_WRONLY)"); perror("open(O_WRONLY)");
return EXIT_FAILURE; return EXIT_FAILURE;
...@@ -41,6 +99,18 @@ int main(int argc, char **argv) ...@@ -41,6 +99,18 @@ int main(int argc, char **argv)
close(fd); close(fd);
rc = get_immutable(path);
if (rc < 0) {
perror("ioctl(FS_IOC_GETFLAGS)");
return EXIT_FAILURE;
} else if (rc) {
rc = set_immutable(path, 0);
if (rc < 0) {
perror("ioctl(FS_IOC_SETFLAGS)");
return EXIT_FAILURE;
}
}
fd = open(path, O_RDONLY); fd = open(path, O_RDONLY);
if (fd < 0) { if (fd < 0) {
perror("open"); perror("open");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment