Messages in this thread |  | | Date | Sun, 4 Dec 2022 03:36:35 +0000 | From | Al Viro <> | Subject | Re: PATCH [1/2] gamecube/wii: graphic quantization registers driver (GQR) |
| |
On Sun, Dec 04, 2022 at 02:06:06PM +1100, Zopolis0 wrote:
> +static u32 gqr_values[8]; > +static struct ctl_table_header *gqr_table_header; > + > +#define SPR_GQR0 912 > +#define SPR_GQR1 913 > +#define SPR_GQR2 914 > +#define SPR_GQR3 915 > +#define SPR_GQR4 916 > +#define SPR_GQR5 917 > +#define SPR_GQR6 918 > +#define SPR_GQR7 919 > + > +#define MFSPR_CASE(i) case (i): (*((u32 *)table->data) = mfspr(SPR_GQR##i)) > +#define MTSPR_CASE(i) case (i): mtspr(SPR_GQR##i, *((u32 *)table->data)) > + > +static int proc_dogqr(ctl_table *table, int write, struct file *file, > + void __user *buffer, size_t *lenp, loff_t *ppos) > +{ > + int r; > + > + if (!write) { /* if they are reading, update the variable */ > + switch (table->data - (void *)gqr_values) {
That looks very fishy. First of all, you clearly have table->data set (by DECLARE_GQR below) to gqr_values + <interger from 0 to 7>. Trivial C quiz: what would the value of (void *)(gqr_values + 7) - (void *)gqr_values be, if gqr_values is declared as an array of unsigned int?
IOW, the values in that switch are wrong. If anything, you want (u32 *)table->data - gqr_values. What's more, SPR_GQR<n> is simply 912 + n, innit? So all the crap with switches and ## is pointless - that thing should simply be
unsigned reg = (u32 *)table->data - gqr_values;
if (WARN_ON_ONCE(reg > 7)) return -EINVAL;
if (!write) gqr_values[reg] = mfspr(SPR_GQR0 + reg);
r = proc_dointvec(table, write, buffer, lenp, ppos);
if (!r && write) /* if they are writing, update the reg */ mtspr(SPR_GQR0 + reg, gqr_values[reg]);
return r;
|  |