From: Joe Perches <joe@perches.com>
To: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>,
"Noralf Trønnes" <noralf@tronnes.org>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] staging: fbtft: fix out of bound access
Date: Mon, 08 Jun 2015 08:57:08 -0700 [thread overview]
Message-ID: <1433779028.2742.10.camel@perches.com> (raw)
In-Reply-To: <20150608145201.GA15145@sudip-PC>
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) \
prev parent reply other threads:[~2015-06-08 15:57 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-04 13:34 [PATCH v2] staging: fbtft: fix out of bound access Sudip Mukherjee
2015-06-04 20:48 ` Joe Perches
2015-06-05 4:52 ` Sudip Mukherjee
2015-06-05 5:46 ` Joe Perches
2015-06-08 14:52 ` Sudip Mukherjee
2015-06-08 15:57 ` Joe Perches [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1433779028.2742.10.camel@perches.com \
--to=joe@perches.com \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=noralf@tronnes.org \
--cc=sudipm.mukherjee@gmail.com \
--cc=thomas.petazzoni@free-electrons.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.