| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 14 Nov 2014 18:07:16 +0100
From: Petr Mladek <pmladek@...e.cz>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Jiri Kosina <jkosina@...e.cz>
Subject: Re: [RFC][PATCH 17/23 v4] tracing: Have seq_buf use full buffer
On Thu 2014-11-13 20:13:01, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.org>
>
> Currently seq_buf is full when all but one byte of the buffer is
> filled. Change it so that the seq_buf is full when all of the
> buffer is filled.
>
> Some of the functions would fill the buffer completely and report
> everything was fine. This was inconsistent with the max of size - 1.
> Changing this to be max of size makes all functions consistent.
>
> Link: http://lkml.kernel.org/r/20141104160222.502133196@goodmis.org
>
> Tested-by: Jiri Kosina <jkosina@...e.cz>
> Acked-by: Jiri Kosina <jkosina@...e.cz>
> Reviewed-by: Petr Mladek <pmladek@...e.cz>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.org>
Hmm, we should not apply this patch before we fix all other locations
accessing seq.len. We need to make sure that they do not access
outside of the buffer when seq.len = seq.size + 1.
See my comments for "[RFC][PATCH 13/23 v4] tracing: Create seq_buf
layer in trace_seq"
BTW: Are these patches applied in some public branch, please? The
patch series is getting long. I would like to see it applied but I do
not want to copy all the patches and apply manually :-)
Best Regards,
Petr
PS: I will need to go in a while. I am not sure that I will be able to
review the whole patchset before the weekend. I do the review in the
order of patches. I sent reply only when I had something to add. The
non-commented other patches (< 17) looks fine to me.
> ---
> include/linux/seq_buf.h | 6 +++---
> kernel/trace/seq_buf.c | 19 +++++++++++--------
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> index 5d91262433e2..581c1fc733c3 100644
> --- a/include/linux/seq_buf.h
> +++ b/include/linux/seq_buf.h
> @@ -43,13 +43,13 @@ seq_buf_init(struct seq_buf *s, unsigned char *buf, unsigned int size)
> static inline bool
> seq_buf_has_overflowed(struct seq_buf *s)
> {
> - return s->len == s->size;
> + return s->len > s->size;
> }
>
> static inline void
> seq_buf_set_overflow(struct seq_buf *s)
> {
> - s->len = s->size;
> + s->len = s->size + 1;
> }
>
> /*
> @@ -61,7 +61,7 @@ seq_buf_buffer_left(struct seq_buf *s)
> if (seq_buf_has_overflowed(s))
> return 0;
>
> - return (s->size - 1) - s->len;
> + return s->size - s->len;
> }
>
> extern __printf(2, 3)
> diff --git a/kernel/trace/seq_buf.c b/kernel/trace/seq_buf.c
> index 7dac34d1235b..9d3bb64dca31 100644
> --- a/kernel/trace/seq_buf.c
> +++ b/kernel/trace/seq_buf.c
> @@ -17,7 +17,7 @@
> #include <linux/seq_buf.h>
>
> /* How much buffer is written? */
> -#define SEQ_BUF_USED(s) min((s)->len, (s)->size - 1)
> +#define SEQ_BUF_USED(s) min((s)->len, (s)->size)
>
> /**
> * seq_buf_print_seq - move the contents of seq_buf into a seq_file
> @@ -51,7 +51,7 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)
>
> if (s->len < s->size) {
> len = vsnprintf(s->buffer + s->len, s->size - s->len, fmt, args);
> - if (s->len + len < s->size) {
> + if (s->len + len <= s->size) {
> s->len += len;
> return 0;
> }
> @@ -100,8 +100,11 @@ int seq_buf_bitmask(struct seq_buf *s, const unsigned long *maskp,
> WARN_ON(s->size == 0);
>
> /*
> - * The last byte of the buffer is used to determine if we
> - * overflowed or not.
> + * Note, because bitmap_scnprintf() only returns the number of bytes
> + * written and not the number that would be written, we use the last
> + * byte of the buffer to let us know if we overflowed. There's a small
> + * chance that the bitmap could have fit exactly inside the buffer, but
> + * it's not that critical if that does happen.
> */
> if (len > 1) {
> ret = bitmap_scnprintf(s->buffer + s->len, len, maskp, nmaskbits);
> @@ -140,7 +143,7 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)
>
> if (s->len < s->size) {
> ret = bstr_printf(s->buffer + s->len, len, fmt, binary);
> - if (s->len + ret < s->size) {
> + if (s->len + ret <= s->size) {
> s->len += ret;
> return 0;
> }
> @@ -164,7 +167,7 @@ int seq_buf_puts(struct seq_buf *s, const char *str)
>
> WARN_ON(s->size == 0);
>
> - if (s->len + len < s->size) {
> + if (s->len + len <= s->size) {
> memcpy(s->buffer + s->len, str, len);
> s->len += len;
> return 0;
> @@ -186,7 +189,7 @@ int seq_buf_putc(struct seq_buf *s, unsigned char c)
> {
> WARN_ON(s->size == 0);
>
> - if (s->len + 1 < s->size) {
> + if (s->len + 1 <= s->size) {
> s->buffer[s->len++] = c;
> return 0;
> }
> @@ -210,7 +213,7 @@ int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len)
> {
> WARN_ON(s->size == 0);
>
> - if (s->len + len < s->size) {
> + if (s->len + len <= s->size) {
> memcpy(s->buffer + s->len, mem, len);
> s->len += len;
> return 0;
> --
> 2.1.1
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists