| Message ID | 20180423173332.561489-2-tom.hromatka@oracle.com |
|---|---|
| State | Superseded |
| Delegated to: | David Miller |
| Headers | show |
| Series | sparc64: Add privileged ADI driver | expand |
On Mon, Apr 23, 2018 at 11:33:31AM -0600, Tom Hromatka wrote: > SPARC M7 and newer processors utilize ADI to version and > protect memory. This driver is capable of reading/writing > ADI/MCD versions from privileged user space processes. > Addresses in the adi file are mapped linearly to physical > memory at a ratio of 1:adi_blksz. Thus, a read (or write) > of offset K in the file operates upon the ADI version at > physical address K * adi_blksz. The version information > is encoded as one version per byte. Intended consumers > are makedumpfile and crash. What do you mean by "crash"? Should this tie into the pstore infrastructure, or is this just a userspace thing? Just curious. Minor code comments below now that the license stuff is correct, I decided to read the code :) > +#include <linux/kernel.h> > +#include <linux/miscdevice.h> > +#include <linux/module.h> > +#include <linux/proc_fs.h> > +#include <linux/slab.h> > +#include <linux/uaccess.h> > +#include <asm/asi.h> > + > +#define MODULE_NAME "adi" What's wrong with KBUILD_MODNAME? Just use that instead of MODULE_NAME later on in the file. > +#define MAX_BUF_SZ 4096 PAGE_SIZE? Just curious. > + > +static int adi_open(struct inode *inode, struct file *file) > +{ > + file->f_mode |= FMODE_UNSIGNED_OFFSET; That's odd, why? > + return 0; > +} > + > +static int read_mcd_tag(unsigned long addr) > +{ > + long err; > + int ver; > + > + __asm__ __volatile__( > + "1: ldxa [%[addr]] %[asi], %[ver]\n" > + " mov 0, %[err]\n" > + "2:\n" > + " .section .fixup,#alloc,#execinstr\n" > + " .align 4\n" > + "3: sethi %%hi(2b), %%g1\n" > + " jmpl %%g1 + %%lo(2b), %%g0\n" > + " mov %[invalid], %[err]\n" > + " .previous\n" > + " .section __ex_table, \"a\"\n" > + " .align 4\n" > + " .word 1b, 3b\n" > + " .previous\n" > + : [ver] "=r" (ver), [err] "=r" (err) > + : [addr] "r" (addr), [invalid] "i" (EFAULT), > + [asi] "i" (ASI_MCD_REAL) > + : "memory", "g1" > + ); > + > + if (err) > + return -EFAULT; > + else > + return ver; > +} > + > +static ssize_t adi_read(struct file *file, char __user *buf, > + size_t count, loff_t *offp) > +{ > + size_t ver_buf_sz, bytes_read = 0; > + int ver_buf_idx = 0; > + loff_t offset; > + u8 *ver_buf; > + ssize_t ret; > + > + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); > + ver_buf = kmalloc(ver_buf_sz, GFP_KERNEL); > + if (!ver_buf) > + return -ENOMEM; > + > + offset = (*offp) * adi_blksize(); > + > + while (bytes_read < count) { > + ret = read_mcd_tag(offset); > + if (ret < 0) > + goto out; > + > + ver_buf[ver_buf_idx] = (u8)ret; Are you sure ret fits in 8 bits here? > + ver_buf_idx++; > + offset += adi_blksize(); > + > + if (ver_buf_idx >= ver_buf_sz) { > + if (copy_to_user(buf + bytes_read, ver_buf, > + ver_buf_sz)) { > + ret = -EFAULT; > + goto out; > + } > + > + bytes_read += ver_buf_sz; > + ver_buf_idx = 0; > + > + ver_buf_sz = min(count - bytes_read, > + (size_t)MAX_BUF_SZ); > + } > + } > + > + (*offp) += bytes_read; > + ret = bytes_read; > +out: > + kfree(ver_buf); > + return ret; > +} > + > +static int set_mcd_tag(unsigned long addr, u8 ver) > +{ > + long err; > + > + __asm__ __volatile__( > + "1: stxa %[ver], [%[addr]] %[asi]\n" > + " mov 0, %[err]\n" > + "2:\n" > + " .section .fixup,#alloc,#execinstr\n" > + " .align 4\n" > + "3: sethi %%hi(2b), %%g1\n" > + " jmpl %%g1 + %%lo(2b), %%g0\n" > + " mov %[invalid], %[err]\n" > + " .previous\n" > + " .section __ex_table, \"a\"\n" > + " .align 4\n" > + " .word 1b, 3b\n" > + " .previous\n" > + : [err] "=r" (err) > + : [ver] "r" (ver), [addr] "r" (addr), > + [invalid] "i" (EFAULT), [asi] "i" (ASI_MCD_REAL) > + : "memory", "g1" > + ); > + > + if (err) > + return -EFAULT; > + else > + return ver; > +} > + > +static ssize_t adi_write(struct file *file, const char __user *buf, > + size_t count, loff_t *offp) > +{ > + size_t ver_buf_sz, bytes_written = 0; > + loff_t offset; > + u8 *ver_buf; > + ssize_t ret; > + int i; > + > + if (count <= 0) > + return -EINVAL; > + > + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); > + ver_buf = kmalloc(ver_buf_sz, (size_t)GFP_KERNEL); (size_t) for GFP_KERNEL? That's really odd looking. > + if (!ver_buf) > + return -ENOMEM; > + > + offset = (*offp) * adi_blksize(); > + > + do { > + if (copy_from_user(ver_buf, &buf[bytes_written], > + ver_buf_sz)) { > + ret = -EFAULT; > + goto out; > + } > + > + for (i = 0; i < ver_buf_sz; i++) { > + ret = set_mcd_tag(offset, ver_buf[i]); > + if (ret < 0) > + goto out; > + > + offset += adi_blksize(); > + } > + > + bytes_written += ver_buf_sz; > + ver_buf_sz = min(count - bytes_written, (size_t)MAX_BUF_SZ); > + } while (bytes_written < count); > + > + (*offp) += bytes_written; > + ret = bytes_written; > +out: > + __asm__ __volatile__("membar #Sync"); > + kfree(ver_buf); > + return ret; > +} > + > +static loff_t adi_llseek(struct file *file, loff_t offset, int whence) > +{ > + loff_t ret = -EINVAL; > + > + switch (whence) { > + case SEEK_END: > + case SEEK_DATA: > + case SEEK_HOLE: > + /* unsupported */ > + return -EINVAL; > + case SEEK_CUR: > + if (offset == 0) > + return file->f_pos; > + > + offset += file->f_pos; > + break; > + case SEEK_SET: > + break; > + } > + > + if (offset != file->f_pos) { > + file->f_pos = offset; > + file->f_version = 0; > + ret = offset; > + } > + > + return ret; > +} Why can't you use default_llseek here? Why do you not allow HOLE and others? Anyway, just tiny questions, all are trivial and not really a big deal if you have tested it on your hardware. I'm guessing this will go through the SPARC tree? If so feel free to add: Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Or if you want/need me to take it through my char/misc tree, just let me know and I can. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/23/2018 11:52 AM, Greg KH wrote: > On Mon, Apr 23, 2018 at 11:33:31AM -0600, Tom Hromatka wrote: >> SPARC M7 and newer processors utilize ADI to version and >> protect memory. This driver is capable of reading/writing >> ADI/MCD versions from privileged user space processes. >> Addresses in the adi file are mapped linearly to physical >> memory at a ratio of 1:adi_blksz. Thus, a read (or write) >> of offset K in the file operates upon the ADI version at >> physical address K * adi_blksz. The version information >> is encoded as one version per byte. Intended consumers >> are makedumpfile and crash. > What do you mean by "crash"? Should this tie into the pstore > infrastructure, or is this just a userspace thing? Just curious. My apologies. I was referring to the crash utility: https://github.com/crash-utility/crash A future commit to store the ADI versions to the pstore would be really cool. I am fearful the amount of ADI data could overwhelm the pstore, though. The latest sparc machines support 4 TB of RAM which could mean several GBs of ADI versions. But storing the ADI versions pertaining to the failing code should be possible. I need to do more research... > > Minor code comments below now that the license stuff is correct, I > decided to read the code :) :) >> +#include <linux/kernel.h> >> +#include <linux/miscdevice.h> >> +#include <linux/module.h> >> +#include <linux/proc_fs.h> >> +#include <linux/slab.h> >> +#include <linux/uaccess.h> >> +#include <asm/asi.h> >> + >> +#define MODULE_NAME "adi" > What's wrong with KBUILD_MODNAME? Just use that instead of MODULE_NAME > later on in the file. Good catch. I'll do that in the next rev of this patch. >> +#define MAX_BUF_SZ 4096 > PAGE_SIZE? Just curious. When a user requests a large read/write in makedumpfile or the crash utility, these tools typically make requests in 4096-sized chunks. I believe you are correct that these operations are based upon page size, but I have not verified. I was hesitant to connect MAX_BUF_SZ to PAGE_SIZE without this verification. I'll look into it more... >> + >> +static int adi_open(struct inode *inode, struct file *file) >> +{ >> + file->f_mode |= FMODE_UNSIGNED_OFFSET; > That's odd, why? sparc64 currently supports 4 TB of RAM (and could support much more in the future). Offsets into this ADI privileged driver are address / 64, but that could change also in the future depending upon cache line sizes. I was afraid that future sparc systems could have very large file offsets. Overkill? > >> + return 0; >> +} >> + >> +static int read_mcd_tag(unsigned long addr) >> +{ >> + long err; >> + int ver; >> + >> + __asm__ __volatile__( >> + "1: ldxa [%[addr]] %[asi], %[ver]\n" >> + " mov 0, %[err]\n" >> + "2:\n" >> + " .section .fixup,#alloc,#execinstr\n" >> + " .align 4\n" >> + "3: sethi %%hi(2b), %%g1\n" >> + " jmpl %%g1 + %%lo(2b), %%g0\n" >> + " mov %[invalid], %[err]\n" >> + " .previous\n" >> + " .section __ex_table, \"a\"\n" >> + " .align 4\n" >> + " .word 1b, 3b\n" >> + " .previous\n" >> + : [ver] "=r" (ver), [err] "=r" (err) >> + : [addr] "r" (addr), [invalid] "i" (EFAULT), >> + [asi] "i" (ASI_MCD_REAL) >> + : "memory", "g1" >> + ); >> + >> + if (err) >> + return -EFAULT; >> + else >> + return ver; >> +} >> + >> +static ssize_t adi_read(struct file *file, char __user *buf, >> + size_t count, loff_t *offp) >> +{ >> + size_t ver_buf_sz, bytes_read = 0; >> + int ver_buf_idx = 0; >> + loff_t offset; >> + u8 *ver_buf; >> + ssize_t ret; >> + >> + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); >> + ver_buf = kmalloc(ver_buf_sz, GFP_KERNEL); >> + if (!ver_buf) >> + return -ENOMEM; >> + >> + offset = (*offp) * adi_blksize(); >> + >> + while (bytes_read < count) { >> + ret = read_mcd_tag(offset); >> + if (ret < 0) >> + goto out; >> + >> + ver_buf[ver_buf_idx] = (u8)ret; > Are you sure ret fits in 8 bits here? Yes, I believe so. read_mcd_tag() will return a negative number on an error - which is checked a couple lines above. Otherwise, the read succeeded which means a valid ADI version was returned. Valid ADI versions are 0 through 16. >> + ver_buf_idx++; >> + offset += adi_blksize(); >> + >> + if (ver_buf_idx >= ver_buf_sz) { >> + if (copy_to_user(buf + bytes_read, ver_buf, >> + ver_buf_sz)) { >> + ret = -EFAULT; >> + goto out; >> + } >> + >> + bytes_read += ver_buf_sz; >> + ver_buf_idx = 0; >> + >> + ver_buf_sz = min(count - bytes_read, >> + (size_t)MAX_BUF_SZ); >> + } >> + } >> + >> + (*offp) += bytes_read; >> + ret = bytes_read; >> +out: >> + kfree(ver_buf); >> + return ret; >> +} >> + >> +static int set_mcd_tag(unsigned long addr, u8 ver) >> +{ >> + long err; >> + >> + __asm__ __volatile__( >> + "1: stxa %[ver], [%[addr]] %[asi]\n" >> + " mov 0, %[err]\n" >> + "2:\n" >> + " .section .fixup,#alloc,#execinstr\n" >> + " .align 4\n" >> + "3: sethi %%hi(2b), %%g1\n" >> + " jmpl %%g1 + %%lo(2b), %%g0\n" >> + " mov %[invalid], %[err]\n" >> + " .previous\n" >> + " .section __ex_table, \"a\"\n" >> + " .align 4\n" >> + " .word 1b, 3b\n" >> + " .previous\n" >> + : [err] "=r" (err) >> + : [ver] "r" (ver), [addr] "r" (addr), >> + [invalid] "i" (EFAULT), [asi] "i" (ASI_MCD_REAL) >> + : "memory", "g1" >> + ); >> + >> + if (err) >> + return -EFAULT; >> + else >> + return ver; >> +} >> + >> +static ssize_t adi_write(struct file *file, const char __user *buf, >> + size_t count, loff_t *offp) >> +{ >> + size_t ver_buf_sz, bytes_written = 0; >> + loff_t offset; >> + u8 *ver_buf; >> + ssize_t ret; >> + int i; >> + >> + if (count <= 0) >> + return -EINVAL; >> + >> + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); >> + ver_buf = kmalloc(ver_buf_sz, (size_t)GFP_KERNEL); > (size_t) for GFP_KERNEL? That's really odd looking. Agreed. Good find. > >> + if (!ver_buf) >> + return -ENOMEM; >> + >> + offset = (*offp) * adi_blksize(); >> + >> + do { >> + if (copy_from_user(ver_buf, &buf[bytes_written], >> + ver_buf_sz)) { >> + ret = -EFAULT; >> + goto out; >> + } >> + >> + for (i = 0; i < ver_buf_sz; i++) { >> + ret = set_mcd_tag(offset, ver_buf[i]); >> + if (ret < 0) >> + goto out; >> + >> + offset += adi_blksize(); >> + } >> + >> + bytes_written += ver_buf_sz; >> + ver_buf_sz = min(count - bytes_written, (size_t)MAX_BUF_SZ); >> + } while (bytes_written < count); >> + >> + (*offp) += bytes_written; >> + ret = bytes_written; >> +out: >> + __asm__ __volatile__("membar #Sync"); >> + kfree(ver_buf); >> + return ret; >> +} >> + >> +static loff_t adi_llseek(struct file *file, loff_t offset, int whence) >> +{ >> + loff_t ret = -EINVAL; >> + >> + switch (whence) { >> + case SEEK_END: >> + case SEEK_DATA: >> + case SEEK_HOLE: >> + /* unsupported */ >> + return -EINVAL; >> + case SEEK_CUR: >> + if (offset == 0) >> + return file->f_pos; >> + >> + offset += file->f_pos; >> + break; >> + case SEEK_SET: >> + break; >> + } >> + >> + if (offset != file->f_pos) { >> + file->f_pos = offset; >> + file->f_version = 0; >> + ret = offset; >> + } >> + >> + return ret; >> +} > Why can't you use default_llseek here? Why do you not allow HOLE and > others? I believe default_llseek() would work, but I chose not to use it because I haven't tested some cases - like SEEK_HOLE. My ADI changes to makedumpfile and crash utility don't utilize SEEK_HOLE. I felt uncomfortable providing a feature without testing it thoroughly, so I decided to save it for a future patchset. > > Anyway, just tiny questions, all are trivial and not really a big deal > if you have tested it on your hardware. I'm guessing this will go > through the SPARC tree? If so feel free to add: That was my plan since this driver is only applicable to sparc64 machines. But I'm open to however you and Dave M think it would be best to proceed. > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Or if you want/need me to take it through my char/misc tree, just let me > know and I can. Thanks so much for the help. I really appreciate it. Tom > > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe sparclinux" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig index 40947a796666..5ecddc95a698 100644 --- a/drivers/char/Kconfig +++ b/drivers/char/Kconfig @@ -540,5 +540,17 @@ source "drivers/s390/char/Kconfig" source "drivers/char/xillybus/Kconfig" +config ADI + tristate "SPARC Privileged ADI driver" + depends on SPARC64 + default m + help + SPARC M7 and newer processors utilize ADI (Application Data + Integrity) to version and protect memory. This driver provides + read/write access to the ADI versions for privileged processes. + This feature is also known as MCD (Memory Corruption Detection) + and SSM (Silicon Secured Memory). Intended consumers of this + driver include crash and makedumpfile. + endmenu diff --git a/drivers/char/Makefile b/drivers/char/Makefile index c97c768cd1dd..b8d42b4e979b 100644 --- a/drivers/char/Makefile +++ b/drivers/char/Makefile @@ -57,3 +57,4 @@ js-rtc-y = rtc.o obj-$(CONFIG_XILLYBUS) += xillybus/ obj-$(CONFIG_POWERNV_OP_PANEL) += powernv-op-panel.o +obj-$(CONFIG_ADI) += adi.o diff --git a/drivers/char/adi.c b/drivers/char/adi.c new file mode 100644 index 000000000000..062b0d7feda2 --- /dev/null +++ b/drivers/char/adi.c @@ -0,0 +1,240 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Privileged ADI driver for sparc64 + * + * Author: Tom Hromatka <tom.hromatka@oracle.com> + */ +#include <linux/kernel.h> +#include <linux/miscdevice.h> +#include <linux/module.h> +#include <linux/proc_fs.h> +#include <linux/slab.h> +#include <linux/uaccess.h> +#include <asm/asi.h> + +#define MODULE_NAME "adi" +#define MAX_BUF_SZ 4096 + +static int adi_open(struct inode *inode, struct file *file) +{ + file->f_mode |= FMODE_UNSIGNED_OFFSET; + return 0; +} + +static int read_mcd_tag(unsigned long addr) +{ + long err; + int ver; + + __asm__ __volatile__( + "1: ldxa [%[addr]] %[asi], %[ver]\n" + " mov 0, %[err]\n" + "2:\n" + " .section .fixup,#alloc,#execinstr\n" + " .align 4\n" + "3: sethi %%hi(2b), %%g1\n" + " jmpl %%g1 + %%lo(2b), %%g0\n" + " mov %[invalid], %[err]\n" + " .previous\n" + " .section __ex_table, \"a\"\n" + " .align 4\n" + " .word 1b, 3b\n" + " .previous\n" + : [ver] "=r" (ver), [err] "=r" (err) + : [addr] "r" (addr), [invalid] "i" (EFAULT), + [asi] "i" (ASI_MCD_REAL) + : "memory", "g1" + ); + + if (err) + return -EFAULT; + else + return ver; +} + +static ssize_t adi_read(struct file *file, char __user *buf, + size_t count, loff_t *offp) +{ + size_t ver_buf_sz, bytes_read = 0; + int ver_buf_idx = 0; + loff_t offset; + u8 *ver_buf; + ssize_t ret; + + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); + ver_buf = kmalloc(ver_buf_sz, GFP_KERNEL); + if (!ver_buf) + return -ENOMEM; + + offset = (*offp) * adi_blksize(); + + while (bytes_read < count) { + ret = read_mcd_tag(offset); + if (ret < 0) + goto out; + + ver_buf[ver_buf_idx] = (u8)ret; + ver_buf_idx++; + offset += adi_blksize(); + + if (ver_buf_idx >= ver_buf_sz) { + if (copy_to_user(buf + bytes_read, ver_buf, + ver_buf_sz)) { + ret = -EFAULT; + goto out; + } + + bytes_read += ver_buf_sz; + ver_buf_idx = 0; + + ver_buf_sz = min(count - bytes_read, + (size_t)MAX_BUF_SZ); + } + } + + (*offp) += bytes_read; + ret = bytes_read; +out: + kfree(ver_buf); + return ret; +} + +static int set_mcd_tag(unsigned long addr, u8 ver) +{ + long err; + + __asm__ __volatile__( + "1: stxa %[ver], [%[addr]] %[asi]\n" + " mov 0, %[err]\n" + "2:\n" + " .section .fixup,#alloc,#execinstr\n" + " .align 4\n" + "3: sethi %%hi(2b), %%g1\n" + " jmpl %%g1 + %%lo(2b), %%g0\n" + " mov %[invalid], %[err]\n" + " .previous\n" + " .section __ex_table, \"a\"\n" + " .align 4\n" + " .word 1b, 3b\n" + " .previous\n" + : [err] "=r" (err) + : [ver] "r" (ver), [addr] "r" (addr), + [invalid] "i" (EFAULT), [asi] "i" (ASI_MCD_REAL) + : "memory", "g1" + ); + + if (err) + return -EFAULT; + else + return ver; +} + +static ssize_t adi_write(struct file *file, const char __user *buf, + size_t count, loff_t *offp) +{ + size_t ver_buf_sz, bytes_written = 0; + loff_t offset; + u8 *ver_buf; + ssize_t ret; + int i; + + if (count <= 0) + return -EINVAL; + + ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ); + ver_buf = kmalloc(ver_buf_sz, (size_t)GFP_KERNEL); + if (!ver_buf) + return -ENOMEM; + + offset = (*offp) * adi_blksize(); + + do { + if (copy_from_user(ver_buf, &buf[bytes_written], + ver_buf_sz)) { + ret = -EFAULT; + goto out; + } + + for (i = 0; i < ver_buf_sz; i++) { + ret = set_mcd_tag(offset, ver_buf[i]); + if (ret < 0) + goto out; + + offset += adi_blksize(); + } + + bytes_written += ver_buf_sz; + ver_buf_sz = min(count - bytes_written, (size_t)MAX_BUF_SZ); + } while (bytes_written < count); + + (*offp) += bytes_written; + ret = bytes_written; +out: + __asm__ __volatile__("membar #Sync"); + kfree(ver_buf); + return ret; +} + +static loff_t adi_llseek(struct file *file, loff_t offset, int whence) +{ + loff_t ret = -EINVAL; + + switch (whence) { + case SEEK_END: + case SEEK_DATA: + case SEEK_HOLE: + /* unsupported */ + return -EINVAL; + case SEEK_CUR: + if (offset == 0) + return file->f_pos; + + offset += file->f_pos; + break; + case SEEK_SET: + break; + } + + if (offset != file->f_pos) { + file->f_pos = offset; + file->f_version = 0; + ret = offset; + } + + return ret; +} + +static const struct file_operations adi_fops = { + .owner = THIS_MODULE, + .llseek = adi_llseek, + .open = adi_open, + .read = adi_read, + .write = adi_write, +}; + +static struct miscdevice adi_miscdev = { + .minor = MISC_DYNAMIC_MINOR, + .name = MODULE_NAME, + .fops = &adi_fops, +}; + +static int __init adi_init(void) +{ + if (!adi_capable()) + return -EPERM; + + return misc_register(&adi_miscdev); +} + +static void __exit adi_exit(void) +{ + misc_deregister(&adi_miscdev); +} + +module_init(adi_init); +module_exit(adi_exit); + +MODULE_AUTHOR("Tom Hromatka <tom.hromatka@oracle.com>"); +MODULE_DESCRIPTION("Privileged interface to ADI"); +MODULE_VERSION("1.0"); +MODULE_LICENSE("GPL v2");