From: "Bruno Prémont" <bonbons@linux-vserver.org> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: Jiri Kosina <jkosina@suse.cz>, linux-input@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] HID: picoLCD: off by one in dump_buff_as_hex() Date: Wed, 19 Sep 2012 19:35:35 +0000 [thread overview] Message-ID: <20120919213535.34712fb5@neptune.home> (raw) In-Reply-To: <20120917225437.6f2847ee@neptune.home> Dan, What's your opinion on below alternative patch? In addition to yours it makes would-overflow visible. It does not check for output buffer having non-zero size but as callers are local with #defined buffer size I don't think that would be needed. Author: Bruno Prémont <bonbons@linux-vserver.org> Date: Wed Sep 19 21:18:10 2012 +0200 Subject: HID: picoLCD: bounds check in dump_buff_as_hex() Make sure we keep enough space for terminating NUL character after last newline. If we have too much data, replace last byte with '.'s to make overflow visible. Using hex_dump_to_buffer() is not interesting as it adds more overhead and does not append the trailing linefeed. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> --- drivers/hid/hid-picolcd_debugfs.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c index 868853a..c5c2fd9 100644 --- a/drivers/hid/hid-picolcd_debugfs.c +++ b/drivers/hid/hid-picolcd_debugfs.c @@ -381,16 +381,16 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data, const size_t data_len) { int i, j; - for (i = j = 0; i < data_len && j + 3 < dst_sz; i++) { + for (i = j = 0; i < data_len && j + 4 < dst_sz; i++) { dst[j++] = hex_asc[(data[i] >> 4) & 0x0f]; dst[j++] = hex_asc[data[i] & 0x0f]; dst[j++] = ' '; } - if (j < dst_sz) { - dst[j--] = '\0'; - dst[j] = '\n'; - } else - dst[j] = '\0'; + dst[j] = '\0'; + if (j > 0) + dst[j-1] = '\n'; + if (i < data_len && j > 2) + dst[j-2] = dst[j-3] = '.'; } void picolcd_debug_out_report(struct picolcd_data *data,
WARNING: multiple messages have this Message-ID (diff)
From: "Bruno Prémont" <bonbons@linux-vserver.org> To: Dan Carpenter <dan.carpenter@oracle.com> Cc: Jiri Kosina <jkosina@suse.cz>, linux-input@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [patch] HID: picoLCD: off by one in dump_buff_as_hex() Date: Wed, 19 Sep 2012 21:35:35 +0200 [thread overview] Message-ID: <20120919213535.34712fb5@neptune.home> (raw) In-Reply-To: <20120917225437.6f2847ee@neptune.home> Dan, What's your opinion on below alternative patch? In addition to yours it makes would-overflow visible. It does not check for output buffer having non-zero size but as callers are local with #defined buffer size I don't think that would be needed. Author: Bruno Prémont <bonbons@linux-vserver.org> Date: Wed Sep 19 21:18:10 2012 +0200 Subject: HID: picoLCD: bounds check in dump_buff_as_hex() Make sure we keep enough space for terminating NUL character after last newline. If we have too much data, replace last byte with '.'s to make overflow visible. Using hex_dump_to_buffer() is not interesting as it adds more overhead and does not append the trailing linefeed. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Bruno Prémont <bonbons@linux-vserver.org> --- drivers/hid/hid-picolcd_debugfs.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-picolcd_debugfs.c b/drivers/hid/hid-picolcd_debugfs.c index 868853a..c5c2fd9 100644 --- a/drivers/hid/hid-picolcd_debugfs.c +++ b/drivers/hid/hid-picolcd_debugfs.c @@ -381,16 +381,16 @@ static void dump_buff_as_hex(char *dst, size_t dst_sz, const u8 *data, const size_t data_len) { int i, j; - for (i = j = 0; i < data_len && j + 3 < dst_sz; i++) { + for (i = j = 0; i < data_len && j + 4 < dst_sz; i++) { dst[j++] = hex_asc[(data[i] >> 4) & 0x0f]; dst[j++] = hex_asc[data[i] & 0x0f]; dst[j++] = ' '; } - if (j < dst_sz) { - dst[j--] = '\0'; - dst[j] = '\n'; - } else - dst[j] = '\0'; + dst[j] = '\0'; + if (j > 0) + dst[j-1] = '\n'; + if (i < data_len && j > 2) + dst[j-2] = dst[j-3] = '.'; } void picolcd_debug_out_report(struct picolcd_data *data, -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-09-19 19:35 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2012-09-14 11:04 [patch] HID: picoLCD: off by one in dump_buff_as_hex() Dan Carpenter 2012-09-14 11:04 ` Dan Carpenter 2012-09-17 20:54 ` Bruno Prémont 2012-09-17 20:54 ` Bruno Prémont 2012-09-19 19:35 ` Bruno Prémont [this message] 2012-09-19 19:35 ` Bruno Prémont 2012-09-22 12:55 ` Dan Carpenter 2012-09-22 12:55 ` Dan Carpenter 2012-09-24 21:07 ` Jiri Kosina 2012-09-24 21:07 ` Jiri Kosina
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=20120919213535.34712fb5@neptune.home \ --to=bonbons@linux-vserver.org \ --cc=dan.carpenter@oracle.com \ --cc=jkosina@suse.cz \ --cc=kernel-janitors@vger.kernel.org \ --cc=linux-input@vger.kernel.org \ /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: linkBe 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.