pstore: pass allocated memory region back to caller
The buf_lock cannot be held while populating the inodes, so make the backend pass forward an allocated and filled buffer instead. This solves the following backtrace. The effect is that "buf" is only ever used to notify the backends that something was written to it, and shouldn't be used in the read path. To replace the buf_lock during the read path, isolate the open/read/close loop with a separate mutex to maintain serialized access to the backend. Note that is is up to the pstore backend to cope if the (*write)() path is called in the middle of the read path. [ 59.691019] BUG: sleeping function called from invalid context at .../mm/slub.c:847 [ 59.691019] in_atomic(): 0, irqs_disabled(): 1, pid: 1819, name: mount [ 59.691019] Pid: 1819, comm: mount Not tainted 3.0.8 #1 [ 59.691019] Call Trace: [ 59.691019] [<810252d5>] __might_sleep+0xc3/0xca [ 59.691019] [<810a26e6>] kmem_cache_alloc+0x32/0xf3 [ 59.691019] [<810b53ac>] ? __d_lookup_rcu+0x6f/0xf4 [ 59.691019] [<810b68b1>] alloc_inode+0x2a/0x64 [ 59.691019] [<810b6903>] new_inode+0x18/0x43 [ 59.691019] [<81142447>] pstore_get_inode.isra.1+0x11/0x98 [ 59.691019] [<81142623>] pstore_mkfile+0xae/0x26f [ 59.691019] [<810a2a66>] ? kmem_cache_free+0x19/0xb1 [ 59.691019] [<8116c821>] ? ida_get_new_above+0x140/0x158 [ 59.691019] [<811708ea>] ? __init_rwsem+0x1e/0x2c [ 59.691019] [<810b67e8>] ? inode_init_always+0x111/0x1b0 [ 59.691019] [<8102127e>] ? should_resched+0xd/0x27 [ 59.691019] [<8137977f>] ? _cond_resched+0xd/0x21 [ 59.691019] [<81142abf>] pstore_get_records+0x52/0xa7 [ 59.691019] [<8114254b>] pstore_fill_super+0x7d/0x91 [ 59.691019] [<810a7ff5>] mount_single+0x46/0x82 [ 59.691019] [<8114231a>] pstore_mount+0x15/0x17 [ 59.691019] [<811424ce>] ? pstore_get_inode.isra.1+0x98/0x98 [ 59.691019] [<810a8199>] mount_fs+0x5a/0x12d [ 59.691019] [<810b9174>] ? alloc_vfsmnt+0xa4/0x14a [ 59.691019] [<810b9474>] vfs_kern_mount+0x4f/0x7d [ 59.691019] [<810b9d7e>] do_kern_mount+0x34/0xb2 [ 59.691019] [<810bb15f>] do_mount+0x5fc/0x64a [ 59.691019] [<810912fb>] ? strndup_user+0x2e/0x3f [ 59.691019] [<810bb3cb>] sys_mount+0x66/0x99 [ 59.691019] [<8137b537>] sysenter_do_call+0x12/0x26 Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: Tony Luck <tony.luck@intel.com>
This commit is contained in:
parent
cfcfc9eca2
commit
f6f8285132
|
@ -932,7 +932,8 @@ static int erst_check_table(struct acpi_table_erst *erst_tab)
|
||||||
static int erst_open_pstore(struct pstore_info *psi);
|
static int erst_open_pstore(struct pstore_info *psi);
|
||||||
static int erst_close_pstore(struct pstore_info *psi);
|
static int erst_close_pstore(struct pstore_info *psi);
|
||||||
static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
|
static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
|
||||||
struct timespec *time, struct pstore_info *psi);
|
struct timespec *time, char **buf,
|
||||||
|
struct pstore_info *psi);
|
||||||
static int erst_writer(enum pstore_type_id type, u64 *id, unsigned int part,
|
static int erst_writer(enum pstore_type_id type, u64 *id, unsigned int part,
|
||||||
size_t size, struct pstore_info *psi);
|
size_t size, struct pstore_info *psi);
|
||||||
static int erst_clearer(enum pstore_type_id type, u64 id,
|
static int erst_clearer(enum pstore_type_id type, u64 id,
|
||||||
|
@ -986,17 +987,23 @@ static int erst_close_pstore(struct pstore_info *psi)
|
||||||
}
|
}
|
||||||
|
|
||||||
static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
|
static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
|
||||||
struct timespec *time, struct pstore_info *psi)
|
struct timespec *time, char **buf,
|
||||||
|
struct pstore_info *psi)
|
||||||
{
|
{
|
||||||
int rc;
|
int rc;
|
||||||
ssize_t len = 0;
|
ssize_t len = 0;
|
||||||
u64 record_id;
|
u64 record_id;
|
||||||
struct cper_pstore_record *rcd = (struct cper_pstore_record *)
|
struct cper_pstore_record *rcd;
|
||||||
(erst_info.buf - sizeof(*rcd));
|
size_t rcd_len = sizeof(*rcd) + erst_info.bufsize;
|
||||||
|
|
||||||
if (erst_disable)
|
if (erst_disable)
|
||||||
return -ENODEV;
|
return -ENODEV;
|
||||||
|
|
||||||
|
rcd = kmalloc(rcd_len, GFP_KERNEL);
|
||||||
|
if (!rcd) {
|
||||||
|
rc = -ENOMEM;
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
skip:
|
skip:
|
||||||
rc = erst_get_record_id_next(&reader_pos, &record_id);
|
rc = erst_get_record_id_next(&reader_pos, &record_id);
|
||||||
if (rc)
|
if (rc)
|
||||||
|
@ -1004,22 +1011,27 @@ skip:
|
||||||
|
|
||||||
/* no more record */
|
/* no more record */
|
||||||
if (record_id == APEI_ERST_INVALID_RECORD_ID) {
|
if (record_id == APEI_ERST_INVALID_RECORD_ID) {
|
||||||
rc = -1;
|
rc = -EINVAL;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
len = erst_read(record_id, &rcd->hdr, sizeof(*rcd) +
|
len = erst_read(record_id, &rcd->hdr, rcd_len);
|
||||||
erst_info.bufsize);
|
|
||||||
/* The record may be cleared by others, try read next record */
|
/* The record may be cleared by others, try read next record */
|
||||||
if (len == -ENOENT)
|
if (len == -ENOENT)
|
||||||
goto skip;
|
goto skip;
|
||||||
else if (len < 0) {
|
else if (len < sizeof(*rcd)) {
|
||||||
rc = -1;
|
rc = -EIO;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0)
|
if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0)
|
||||||
goto skip;
|
goto skip;
|
||||||
|
|
||||||
|
*buf = kmalloc(len, GFP_KERNEL);
|
||||||
|
if (*buf == NULL) {
|
||||||
|
rc = -ENOMEM;
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
memcpy(*buf, rcd->data, len - sizeof(*rcd));
|
||||||
*id = record_id;
|
*id = record_id;
|
||||||
if (uuid_le_cmp(rcd->sec_hdr.section_type,
|
if (uuid_le_cmp(rcd->sec_hdr.section_type,
|
||||||
CPER_SECTION_TYPE_DMESG) == 0)
|
CPER_SECTION_TYPE_DMESG) == 0)
|
||||||
|
@ -1037,6 +1049,7 @@ skip:
|
||||||
time->tv_nsec = 0;
|
time->tv_nsec = 0;
|
||||||
|
|
||||||
out:
|
out:
|
||||||
|
kfree(rcd);
|
||||||
return (rc < 0) ? rc : (len - sizeof(*rcd));
|
return (rc < 0) ? rc : (len - sizeof(*rcd));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -457,7 +457,8 @@ static int efi_pstore_close(struct pstore_info *psi)
|
||||||
}
|
}
|
||||||
|
|
||||||
static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
|
static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
|
||||||
struct timespec *timespec, struct pstore_info *psi)
|
struct timespec *timespec,
|
||||||
|
char **buf, struct pstore_info *psi)
|
||||||
{
|
{
|
||||||
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
|
efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
|
||||||
struct efivars *efivars = psi->data;
|
struct efivars *efivars = psi->data;
|
||||||
|
@ -478,7 +479,11 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type,
|
||||||
timespec->tv_nsec = 0;
|
timespec->tv_nsec = 0;
|
||||||
get_var_data_locked(efivars, &efivars->walk_entry->var);
|
get_var_data_locked(efivars, &efivars->walk_entry->var);
|
||||||
size = efivars->walk_entry->var.DataSize;
|
size = efivars->walk_entry->var.DataSize;
|
||||||
memcpy(psi->buf, efivars->walk_entry->var.Data, size);
|
*buf = kmalloc(size, GFP_KERNEL);
|
||||||
|
if (*buf == NULL)
|
||||||
|
return -ENOMEM;
|
||||||
|
memcpy(*buf, efivars->walk_entry->var.Data,
|
||||||
|
size);
|
||||||
efivars->walk_entry = list_entry(efivars->walk_entry->list.next,
|
efivars->walk_entry = list_entry(efivars->walk_entry->list.next,
|
||||||
struct efivar_entry, list);
|
struct efivar_entry, list);
|
||||||
return size;
|
return size;
|
||||||
|
|
|
@ -167,6 +167,7 @@ int pstore_register(struct pstore_info *psi)
|
||||||
}
|
}
|
||||||
|
|
||||||
psinfo = psi;
|
psinfo = psi;
|
||||||
|
mutex_init(&psinfo->read_mutex);
|
||||||
spin_unlock(&pstore_lock);
|
spin_unlock(&pstore_lock);
|
||||||
|
|
||||||
if (owner && !try_module_get(owner)) {
|
if (owner && !try_module_get(owner)) {
|
||||||
|
@ -195,30 +196,32 @@ EXPORT_SYMBOL_GPL(pstore_register);
|
||||||
void pstore_get_records(int quiet)
|
void pstore_get_records(int quiet)
|
||||||
{
|
{
|
||||||
struct pstore_info *psi = psinfo;
|
struct pstore_info *psi = psinfo;
|
||||||
|
char *buf = NULL;
|
||||||
ssize_t size;
|
ssize_t size;
|
||||||
u64 id;
|
u64 id;
|
||||||
enum pstore_type_id type;
|
enum pstore_type_id type;
|
||||||
struct timespec time;
|
struct timespec time;
|
||||||
int failed = 0, rc;
|
int failed = 0, rc;
|
||||||
unsigned long flags;
|
|
||||||
|
|
||||||
if (!psi)
|
if (!psi)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
spin_lock_irqsave(&psinfo->buf_lock, flags);
|
mutex_lock(&psi->read_mutex);
|
||||||
rc = psi->open(psi);
|
rc = psi->open(psi);
|
||||||
if (rc)
|
if (rc)
|
||||||
goto out;
|
goto out;
|
||||||
|
|
||||||
while ((size = psi->read(&id, &type, &time, psi)) > 0) {
|
while ((size = psi->read(&id, &type, &time, &buf, psi)) > 0) {
|
||||||
rc = pstore_mkfile(type, psi->name, id, psi->buf, (size_t)size,
|
rc = pstore_mkfile(type, psi->name, id, buf, (size_t)size,
|
||||||
time, psi);
|
time, psi);
|
||||||
|
kfree(buf);
|
||||||
|
buf = NULL;
|
||||||
if (rc && (rc != -EEXIST || !quiet))
|
if (rc && (rc != -EEXIST || !quiet))
|
||||||
failed++;
|
failed++;
|
||||||
}
|
}
|
||||||
psi->close(psi);
|
psi->close(psi);
|
||||||
out:
|
out:
|
||||||
spin_unlock_irqrestore(&psinfo->buf_lock, flags);
|
mutex_unlock(&psi->read_mutex);
|
||||||
|
|
||||||
if (failed)
|
if (failed)
|
||||||
printk(KERN_WARNING "pstore: failed to load %d record(s) from '%s'\n",
|
printk(KERN_WARNING "pstore: failed to load %d record(s) from '%s'\n",
|
||||||
|
|
|
@ -35,10 +35,12 @@ struct pstore_info {
|
||||||
spinlock_t buf_lock; /* serialize access to 'buf' */
|
spinlock_t buf_lock; /* serialize access to 'buf' */
|
||||||
char *buf;
|
char *buf;
|
||||||
size_t bufsize;
|
size_t bufsize;
|
||||||
|
struct mutex read_mutex; /* serialize open/read/close */
|
||||||
int (*open)(struct pstore_info *psi);
|
int (*open)(struct pstore_info *psi);
|
||||||
int (*close)(struct pstore_info *psi);
|
int (*close)(struct pstore_info *psi);
|
||||||
ssize_t (*read)(u64 *id, enum pstore_type_id *type,
|
ssize_t (*read)(u64 *id, enum pstore_type_id *type,
|
||||||
struct timespec *time, struct pstore_info *psi);
|
struct timespec *time, char **buf,
|
||||||
|
struct pstore_info *psi);
|
||||||
int (*write)(enum pstore_type_id type, u64 *id,
|
int (*write)(enum pstore_type_id type, u64 *id,
|
||||||
unsigned int part, size_t size, struct pstore_info *psi);
|
unsigned int part, size_t size, struct pstore_info *psi);
|
||||||
int (*erase)(enum pstore_type_id type, u64 id,
|
int (*erase)(enum pstore_type_id type, u64 id,
|
||||||
|
|
Loading…
Reference in a new issue