Skip to content

Commit

Permalink
pstore: Pass record contents instead of copying
Browse files Browse the repository at this point in the history
pstore_mkfile() shouldn't have to memcpy the record contents. It can use
the existing copy instead. This adjusts the allocation lifetime management
and renames the contents variable from "data" to "buf" to assist moving to
struct pstore_record in the future.

Signed-off-by: Kees Cook <keescook@chromium.org>
  • Loading branch information
Kees Cook committed Mar 7, 2017
1 parent 7e8cc8d commit 1dfff7d
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 11 deletions.
22 changes: 15 additions & 7 deletions fs/pstore/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct pstore_private {
u64 id;
int count;
ssize_t size;
char data[];
char *buf;
};

struct pstore_ftrace_seq_data {
Expand All @@ -63,6 +63,14 @@ struct pstore_ftrace_seq_data {

#define REC_SIZE sizeof(struct pstore_ftrace_record)

static void free_pstore_private(struct pstore_private *private)
{
if (!private)
return;
kfree(private->buf);
kfree(private);
}

static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos)
{
struct pstore_private *ps = s->private;
Expand Down Expand Up @@ -105,7 +113,7 @@ static int pstore_ftrace_seq_show(struct seq_file *s, void *v)
{
struct pstore_private *ps = s->private;
struct pstore_ftrace_seq_data *data = v;
struct pstore_ftrace_record *rec = (void *)(ps->data + data->off);
struct pstore_ftrace_record *rec = (void *)(ps->buf + data->off);

seq_printf(s, "CPU:%d ts:%llu %08lx %08lx %pf <- %pF\n",
pstore_ftrace_decode_cpu(rec),
Expand Down Expand Up @@ -143,7 +151,7 @@ static ssize_t pstore_file_read(struct file *file, char __user *userbuf,

if (ps->type == PSTORE_TYPE_FTRACE)
return seq_read(file, userbuf, count, ppos);
return simple_read_from_buffer(userbuf, count, ppos, ps->data, ps->size);
return simple_read_from_buffer(userbuf, count, ppos, ps->buf, ps->size);
}

static int pstore_file_open(struct inode *inode, struct file *file)
Expand Down Expand Up @@ -221,7 +229,7 @@ static void pstore_evict_inode(struct inode *inode)
spin_lock_irqsave(&allpstore_lock, flags);
list_del(&p->list);
spin_unlock_irqrestore(&allpstore_lock, flags);
kfree(p);
free_pstore_private(p);
}
}

Expand Down Expand Up @@ -332,7 +340,7 @@ int pstore_mkfile(struct pstore_record *record)
goto fail;
inode->i_mode = S_IFREG | 0444;
inode->i_fop = &pstore_file_operations;
private = kmalloc(sizeof *private + size, GFP_KERNEL);
private = kzalloc(sizeof(*private), GFP_KERNEL);
if (!private)
goto fail_alloc;
private->type = record->type;
Expand Down Expand Up @@ -394,7 +402,7 @@ int pstore_mkfile(struct pstore_record *record)
if (!dentry)
goto fail_lockedalloc;

memcpy(private->data, record->buf, size);
private->buf = record->buf;
inode->i_size = private->size = size;

inode->i_private = private;
Expand All @@ -414,7 +422,7 @@ int pstore_mkfile(struct pstore_record *record)

fail_lockedalloc:
inode_unlock(d_inode(root));
kfree(private);
free_pstore_private(private);
fail_alloc:
iput(inode);

Expand Down
16 changes: 12 additions & 4 deletions fs/pstore/platform.c
Original file line number Diff line number Diff line change
Expand Up @@ -828,14 +828,22 @@ void pstore_get_records(int quiet)
if (psi->open && psi->open(psi))
goto out;

/*
* Backend callback read() allocates record.buf. decompress_record()
* may reallocate record.buf. On success, pstore_mkfile() will keep
* the record.buf, so free it only on failure.
*/
while ((record.size = psi->read(&record)) > 0) {
decompress_record(&record);
rc = pstore_mkfile(&record);
if (rc) {
/* pstore_mkfile() did not take buf, so free it. */
kfree(record.buf);
if (rc != -EEXIST || !quiet)
failed++;
}

if (rc && (rc != -EEXIST || !quiet))
failed++;

kfree(record.buf);
/* Reset for next record. */
memset(&record, 0, sizeof(record));
record.psi = psi;
}
Expand Down

0 comments on commit 1dfff7d

Please sign in to comment.