Re: [PATCH] tpm: factor out the user space mm from tpm_vtpm_set_locality()

From: Stefan Berger
Date: Thu Jun 08 2023 - 11:11:22 EST




On 6/8/23 09:14, Jarkko Sakkinen wrote:
On Wed May 31, 2023 at 8:01 PM EEST, Stefan Berger wrote:




This is swtpm picking up this command with its user buffer.

So, I am not sure at this point what is wrong.

Stefan

The answer was below but in short it is that you have a function that
expects __user * and you don't pass user tagged memory.

There are two functions that expect user tagged memory:

static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
size_t count, loff_t *off)
static ssize_t vtpm_proxy_fops_write(struct file *filp, const char __user *buf,
size_t count, loff_t *off)

the correspond to this interface:

struct file_operations {
struct module *owner;
loff_t (*llseek) (struct file *, loff_t, int);
ssize_t (*read) (struct file *, char __user *, size_t, loff_t *);
ssize_t (*write) (struct file *, const char __user *, size_t, loff_t *);

defined here:

static const struct file_operations vtpm_proxy_fops = {
.owner = THIS_MODULE,
.llseek = no_llseek,
.read = vtpm_proxy_fops_read,
.write = vtpm_proxy_fops_write,

Conversely, I see no other function interfaces in tpm_vtpm_proxy.c where the code would be missing the __user.

Neither do I see any functions where I am passing a __user tagged buffer as parameter that shouldn't have
such a tag on it or the reverse where a plain buffer is passed and it should be a __user tagged buffer.

Stefan


Even tho it is a bug, I think cc to stable is not necessary given that
it is not known to blow up anything. The main problem is that we have
code that does not work according to the expectations.

BR, Jarkko