Re: [PATCH v2] staging: fbtft: fix out of bound access
From: Joe Perches
Date: Mon Jun 08 2015 - 11:57:33 EST
On Mon, 2015-06-08 at 20:22 +0530, Sudip Mukherjee wrote:
> On Thu, Jun 04, 2015 at 10:46:51PM -0700, Joe Perches wrote:
> > On Fri, 2015-06-05 at 10:22 +0530, Sudip Mukherjee wrote:
> > > On Thu, Jun 04, 2015 at 01:48:31PM -0700, Joe Perches wrote:
> > []
> <snip>
> > I looked at it a bit more and there's a macro that calls
> > write_register so there are actually many more call sites.
> >
> > It's a bit non trivial to change the macro as all the
> > called (*write_register) functions would need changing
> > and these functions use va_list.
> >
> > Maybe if you _really_ feel like it, but it's a bit of work.
> Hi Joe,
> I was doing this one today, and just changed write_reg8_bus8 to test.
> but when started compiling I found out another variation:
> #define write_reg(par, ...) \
> par->fbtftops.write_register(par, NUMARGS(__VA_ARGS__), __VA_ARGS__)
>
> and there are only 870 calls to write_reg. :(
> I was making it like void write_reg8_bus8(struct fbtft_par *par, int len, int *sbuf)
> but if i have to add an integer array to the places where write_reg is called
> it will become a big change. Any simple idea?
No, because each function will need to be changed.
Maybe something like this is a start, but it doesn't
compile properly because more functions need to be
changed and I haven't tested it.
drivers/staging/fbtft/fbtft-bus.c | 121 +++++++++++++++++--------------------
drivers/staging/fbtft/fbtft-core.c | 36 +----------
drivers/staging/fbtft/fbtft.h | 16 ++---
3 files changed, 68 insertions(+), 105 deletions(-)
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 912c632..292564e 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -13,79 +13,74 @@
*
*****************************************************************************/
-#define define_fbtft_write_reg(func, type, modifier) \
-void func(struct fbtft_par *par, int len, ...) \
-{ \
- va_list args; \
- int i, ret; \
- int offset = 0; \
- type *buf = (type *)par->buf; \
- \
- if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) { \
- va_start(args, len); \
- for (i = 0; i < len; i++) { \
- buf[i] = (type)va_arg(args, unsigned int); \
- } \
- va_end(args); \
- fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par, par->info->device, type, buf, len, "%s: ", __func__); \
- } \
- \
- va_start(args, len); \
- \
- if (par->startbyte) { \
- *(u8 *)par->buf = par->startbyte; \
- buf = (type *)(par->buf + 1); \
- offset = 1; \
- } \
- \
- *buf = modifier((type)va_arg(args, unsigned int)); \
- if (par->gpio.dc != -1) \
- gpio_set_value(par->gpio.dc, 0); \
- ret = par->fbtftops.write(par, par->buf, sizeof(type)+offset); \
- if (ret < 0) { \
- va_end(args); \
- dev_err(par->info->device, "%s: write() failed and returned %d\n", __func__, ret); \
- return; \
- } \
- len--; \
- \
- if (par->startbyte) \
- *(u8 *)par->buf = par->startbyte | 0x2; \
- \
- if (len) { \
- i = len; \
- while (i--) { \
- *buf++ = modifier((type)va_arg(args, unsigned int)); \
- } \
- if (par->gpio.dc != -1) \
- gpio_set_value(par->gpio.dc, 1); \
- ret = par->fbtftops.write(par, par->buf, len * (sizeof(type)+offset)); \
- if (ret < 0) { \
- va_end(args); \
- dev_err(par->info->device, "%s: write() failed and returned %d\n", __func__, ret); \
- return; \
- } \
- } \
- va_end(args); \
-} \
+#define define_fbtft_write_reg(func, type, modifier) \
+void func(struct fbtft_par *par, int len, const int *regs) \
+{ \
+ int i, ret; \
+ int offset = 0; \
+ type *buf = (type *)par->buf; \
+ \
+ if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) { \
+ for (i = 0; i < len; i++) \
+ buf[i] = (type)regs[i]; \
+ fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par, \
+ par->info->device, type, buf, len, \
+ "%s: ", __func__); \
+ } \
+ \
+ if (par->startbyte) { \
+ *(u8 *)par->buf = par->startbyte; \
+ buf = (type *)(par->buf + 1); \
+ offset = 1; \
+ } \
+ \
+ *buf = modifier((type)regs[0]); \
+ if (par->gpio.dc != -1) \
+ gpio_set_value(par->gpio.dc, 0); \
+ ret = par->fbtftops.write(par, par->buf, sizeof(type)+offset); \
+ if (ret < 0) { \
+ dev_err(par->info->device, \
+ "%s: write() failed and returned %d\n", \
+ __func__, ret); \
+ return; \
+ } \
+ len--; \
+ \
+ if (par->startbyte) \
+ *(u8 *)par->buf = par->startbyte | 0x2; \
+ \
+ if (len) { \
+ i = len; \
+ while (i--) { \
+ *buf++ = modifier((type)regs[len - i]); \
+ } \
+ if (par->gpio.dc != -1) \
+ gpio_set_value(par->gpio.dc, 1); \
+ ret = par->fbtftops.write(par, par->buf, \
+ len * (sizeof(type)+offset)); \
+ if (ret < 0) { \
+ dev_err(par->info->device, \
+ "%s: write() failed and returned %d\n", \
+ __func__, ret); \
+ return; \
+ } \
+ } \
+} \
EXPORT_SYMBOL(func);
define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, )
define_fbtft_write_reg(fbtft_write_reg16_bus8, u16, cpu_to_be16)
define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, )
-void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
+void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, const int *regs)
{
- va_list args;
int i, ret;
int pad = 0;
u16 *buf = (u16 *)par->buf;
if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {
- va_start(args, len);
for (i = 0; i < len; i++)
- *(((u8 *)buf) + i) = (u8)va_arg(args, unsigned int);
- va_end(args);
+ *(((u8 *)buf) + i) = (u8)regs[i];
fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par,
par->info->device, u8, buf, len, "%s: ", __func__);
}
@@ -100,14 +95,12 @@ void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
*buf++ = 0x000;
}
- va_start(args, len);
- *buf++ = (u8)va_arg(args, unsigned int);
+ *buf++ = (u8)regs[len - 1];
i = len - 1;
while (i--) {
- *buf = (u8)va_arg(args, unsigned int);
+ *buf = (u8)regs[i];
*buf++ |= 0x100; /* dc=1 */
}
- va_end(args);
ret = par->fbtftops.write(par, par->buf, (len + pad) * sizeof(u16));
if (ret < 0) {
dev_err(par->info->device,
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index ce64521..0d925f5 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -1102,23 +1102,7 @@ static int fbtft_init_display_dt(struct fbtft_par *par)
fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
"init: write_register:%s\n", msg);
- par->fbtftops.write_register(par, i,
- buf[0], buf[1], buf[2], buf[3],
- buf[4], buf[5], buf[6], buf[7],
- buf[8], buf[9], buf[10], buf[11],
- buf[12], buf[13], buf[14], buf[15],
- buf[16], buf[17], buf[18], buf[19],
- buf[20], buf[21], buf[22], buf[23],
- buf[24], buf[25], buf[26], buf[27],
- buf[28], buf[29], buf[30], buf[31],
- buf[32], buf[33], buf[34], buf[35],
- buf[36], buf[37], buf[38], buf[39],
- buf[40], buf[41], buf[42], buf[43],
- buf[44], buf[45], buf[46], buf[47],
- buf[48], buf[49], buf[50], buf[51],
- buf[52], buf[53], buf[54], buf[55],
- buf[56], buf[57], buf[58], buf[59],
- buf[60], buf[61], buf[62], buf[63]);
+ par->fbtftops.write_register(par, i, buf);
} else if (val & FBTFT_OF_INIT_DELAY) {
fbtft_par_dbg(DEBUG_INIT_DISPLAY, par,
"init: msleep(%u)\n", val & 0xFFFF);
@@ -1217,23 +1201,7 @@ int fbtft_init_display(struct fbtft_par *par)
}
buf[j++] = par->init_sequence[i++];
}
- par->fbtftops.write_register(par, j,
- buf[0], buf[1], buf[2], buf[3],
- buf[4], buf[5], buf[6], buf[7],
- buf[8], buf[9], buf[10], buf[11],
- buf[12], buf[13], buf[14], buf[15],
- buf[16], buf[17], buf[18], buf[19],
- buf[20], buf[21], buf[22], buf[23],
- buf[24], buf[25], buf[26], buf[27],
- buf[28], buf[29], buf[30], buf[31],
- buf[32], buf[33], buf[34], buf[35],
- buf[36], buf[37], buf[38], buf[39],
- buf[40], buf[41], buf[42], buf[43],
- buf[44], buf[45], buf[46], buf[47],
- buf[48], buf[49], buf[50], buf[51],
- buf[52], buf[53], buf[54], buf[55],
- buf[56], buf[57], buf[58], buf[59],
- buf[60], buf[61], buf[62], buf[63]);
+ par->fbtftops.write_register(par, j, buf);
break;
case -2:
i++;
diff --git a/drivers/staging/fbtft/fbtft.h b/drivers/staging/fbtft/fbtft.h
index 9fd98cb..09d0ce2 100644
--- a/drivers/staging/fbtft/fbtft.h
+++ b/drivers/staging/fbtft/fbtft.h
@@ -85,7 +85,7 @@ struct fbtft_ops {
int (*write)(struct fbtft_par *par, void *buf, size_t len);
int (*read)(struct fbtft_par *par, void *buf, size_t len);
int (*write_vmem)(struct fbtft_par *par, size_t offset, size_t len);
- void (*write_register)(struct fbtft_par *par, int len, ...);
+ void (*write_register)(struct fbtft_par *par, int len, const int *regs);
void (*set_addr_win)(struct fbtft_par *par,
int xs, int ys, int xe, int ye);
@@ -258,8 +258,10 @@ struct fbtft_par {
#define NUMARGS(...) (sizeof((int[]){__VA_ARGS__})/sizeof(int))
-#define write_reg(par, ...) \
- par->fbtftops.write_register(par, NUMARGS(__VA_ARGS__), __VA_ARGS__)
+#define write_reg(par, ...) \
+ par->fbtftops.write_register(par, \
+ NUMARGS(__VA_ARGS__), \
+ ((int[]){__VA_ARGS__}))
/* fbtft-core.c */
extern void fbtft_dbg_hex(const struct device *dev,
@@ -291,10 +293,10 @@ extern int fbtft_write_vmem8_bus8(struct fbtft_par *par, size_t offset, size_t l
extern int fbtft_write_vmem16_bus16(struct fbtft_par *par, size_t offset, size_t len);
extern int fbtft_write_vmem16_bus8(struct fbtft_par *par, size_t offset, size_t len);
extern int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len);
-extern void fbtft_write_reg8_bus8(struct fbtft_par *par, int len, ...);
-extern void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...);
-extern void fbtft_write_reg16_bus8(struct fbtft_par *par, int len, ...);
-extern void fbtft_write_reg16_bus16(struct fbtft_par *par, int len, ...);
+extern void fbtft_write_reg8_bus8(struct fbtft_par *par, int len, const int *regs);
+extern void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, const int *regs);
+extern void fbtft_write_reg16_bus8(struct fbtft_par *par, int len, const int *regs);
+extern void fbtft_write_reg16_bus16(struct fbtft_par *par, int len, const int *regs);
#define FBTFT_REGISTER_DRIVER(_name, _compatible, _display) \
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/