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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20211123113304.19da789b@gandalf.local.home>
Date:   Tue, 23 Nov 2021 11:33:04 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Greg KH <greg@...ah.com>
Cc:     linux-kernel@...r.kernel.org, stable-commits@...r.kernel.org,
        Ingo Molnar <mingo@...hat.com>
Subject: Re: Patch "tracing: Add length protection to histogram string
 copies" has been added to the 5.4-stable tree

On Tue, 23 Nov 2021 13:11:23 +0100
Greg KH <greg@...ah.com> wrote:

> On Sun, Nov 21, 2021 at 06:19:42PM -0500, Sasha Levin wrote:
> > This is a note to let you know that I've just added the patch titled
> > 
> >     tracing: Add length protection to histogram string copies
> > 
> > to the 5.4-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      tracing-add-length-protection-to-histogram-string-co.patch
> > and it can be found in the queue-5.4 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@...r.kernel.org> know about it.
> > 
> > 
> > 
> > commit 0a0e233a0c25593f817b6d9ed48fb2d2fd080870
> > Author: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > Date:   Sun Nov 14 13:28:34 2021 -0500
> > 
> >     tracing: Add length protection to histogram string copies
> >     
> >     [ Upstream commit 938aa33f14657c9ed9deea348b7d6f14b6d69cb7 ]
> >     
> >     The string copies to the histogram storage has a max size of 256 bytes
> >     (defined by MAX_FILTER_STR_VAL). Only the string size of the event field
> >     needs to be copied to the event storage, but no more than what is in the
> >     event storage. Although nothing should be bigger than 256 bytes, there's
> >     no protection against overwriting of the storage if one day there is.
> >     
> >     Copy no more than the destination size, and enforce it.
> >     
> >     Also had to turn MAX_FILTER_STR_VAL into an unsigned int, to keep the
> >     min() comparison of the string sizes of comparable types.
> >     
> >     Link: https://lore.kernel.org/all/CAHk-=wjREUihCGrtRBwfX47y_KrLCGjiq3t6QtoNJpmVrAEb1w@mail.gmail.com/
> >     Link: https://lkml.kernel.org/r/20211114132834.183429a4@rorschach.local.home
> >     
> >     Cc: Ingo Molnar <mingo@...nel.org>
> >     Cc: Andrew Morton <akpm@...ux-foundation.org>
> >     Cc: Tom Zanussi <zanussi@...nel.org>
> >     Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
> >     Reviewed-by: Masami Hiramatsu <mhiramat@...nel.org>
> >     Fixes: 63f84ae6b82b ("tracing/histogram: Do not copy the fixed-size char array field over the field size")
> >     Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> >     Signed-off-by: Sasha Levin <sashal@...nel.org>  
> 
> This commit causes a bunch of build warnings, so odds are the backport
> is incorrect.
> 
> I'll drop it from the 5.4.y queue now.

Well, the above doesn't apply for me to 5.4, but the following appears to
work:

commit 0a0e233a0c25593f817b6d9ed48fb2d2fd080870
Author: Steven Rostedt (VMware) <rostedt@...dmis.org>
Date:   Sun Nov 14 13:28:34 2021 -0500

    tracing: Add length protection to histogram string copies
    
    [ Upstream commit 938aa33f14657c9ed9deea348b7d6f14b6d69cb7 ]
    
    The string copies to the histogram storage has a max size of 256 bytes
    (defined by MAX_FILTER_STR_VAL). Only the string size of the event field
    needs to be copied to the event storage, but no more than what is in the
    event storage. Although nothing should be bigger than 256 bytes, there's
    no protection against overwriting of the storage if one day there is.
    
    Copy no more than the destination size, and enforce it.
    
    Also had to turn MAX_FILTER_STR_VAL into an unsigned int, to keep the
    min() comparison of the string sizes of comparable types.
    
    Link: https://lore.kernel.org/all/CAHk-=wjREUihCGrtRBwfX47y_KrLCGjiq3t6QtoNJpmVrAEb1w@mail.gmail.com/
    Link: https://lkml.kernel.org/r/20211114132834.183429a4@rorschach.local.home
    
    Cc: Ingo Molnar <mingo@...nel.org>
    Cc: Andrew Morton <akpm@...ux-foundation.org>
    Cc: Tom Zanussi <zanussi@...nel.org>
    Reported-by: Linus Torvalds <torvalds@...ux-foundation.org>
    Reviewed-by: Masami Hiramatsu <mhiramat@...nel.org>
    Fixes: 63f84ae6b82b ("tracing/histogram: Do not copy the fixed-size char array field over the field size")
    Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
    Signed-off-by: Sasha Levin <sashal@...nel.org>

Index: linux-test.git/include/linux/trace_events.h
===================================================================
--- linux-test.git.orig/include/linux/trace_events.h
+++ linux-test.git/include/linux/trace_events.h
@@ -425,7 +425,7 @@ struct trace_event_file {
 
 #define PERF_MAX_TRACE_SIZE	2048
 
-#define MAX_FILTER_STR_VAL	256	/* Should handle KSYM_SYMBOL_LEN */
+#define MAX_FILTER_STR_VAL	256U	/* Should handle KSYM_SYMBOL_LEN */
 
 enum event_trigger_type {
 	ETT_NONE		= (0),
Index: linux-test.git/kernel/trace/trace_events_hist.c
===================================================================
--- linux-test.git.orig/kernel/trace/trace_events_hist.c
+++ linux-test.git/kernel/trace/trace_events_hist.c
@@ -3521,8 +3521,10 @@ static inline void __update_field_vars(s
 		if (val->flags & HIST_FIELD_FL_STRING) {
 			char *str = elt_data->field_var_str[j++];
 			char *val_str = (char *)(uintptr_t)var_val;
+			unsigned int size;
 
-			strscpy(str, val_str, STR_VAR_LEN_MAX);
+			size = min(val->size, STR_VAR_LEN_MAX);
+			strscpy(str, val_str, size);
 			var_val = (u64)(uintptr_t)str;
 		}
 		tracing_map_set_var(elt, var_idx, var_val);

-- Steve

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ