linux-kernel - Re: [PATCH] tpm: factor out the user space mm from tpm_vtpm_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <666b8422-3e4f-3d88-1ff7-1f650dd401ce@linux.ibm.com>
Date:   Thu, 8 Jun 2023 11:10:59 -0400
From:   Stefan Berger <stefanb@...ux.ibm.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        linux-integrity@...r.kernel.org
Cc:     Jason Gunthorpe <jgg@...dia.com>,
        Alejandro Cabrera <alejandro.cabreraaldaya@...i.fi>,
        Jarkko Sakkinen <jarkko.sakkinen@...i.fi>,
        stable@...r.kernel.org, Stefan Berger <stefanb@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tpm: factor out the user space mm from
 tpm_vtpm_set_locality()



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
> 
>