browneee added inline comments.
================
Comment at: compiler-rt/lib/dfsan/dfsan_custom.cpp:213
+ char *res = strsep(s, delim);
+ s_label = dfsan_read_label(base, strlen(base));
+ if (res && (res != base)) {
----------------
tkuchta wrote:
> browneee wrote:
> > tkuchta wrote:
> > > browneee wrote:
> > > > The `s_label` represents the taint label for `s` (the pointer).
> > > >
> > > > This line would clobber the taint label of the pointer (`s`) with a
> > > > taint label from `s[0][0..n]`.
> > > >
> > > > I think this line should be deleted.
> > > Agree, s_label represents the taint associated with the **s pointer.
> > > However I am now wondering if that is the taint wich we would like to
> > > return.
> > > For example, if we have
> > > if (flags().strict_data_dependencies) {
> > > *ret_label = res ? s_label : 0;
> > >
> > > We would taint the return value with the value of the pointer, not the
> > > data. It means that if we operate on a string for which the characters
> > > are tainted, but the pointer itself isn't, we are likely going to return
> > > label 0. My understanding was that we are more concerned with the taint
> > > of the data, not the pointer, am I missing something?
> > >
> > Yes, we are usually more concerned with the taint of the data, not the
> > pointer.
> >
> > With strict dependencies:
> > // If the input pointer is tainted, the output pointer would be tainted
> > (because it is derived from the input pointer - maybe the same value).
> > taint(s[0]) == dfsan_read_label(s, sizeof(s)) ====> taint(ret) ==
> > ret_label[0]
> >
> > // If the input data is tainted, the output data would be tainted (because
> > it is derived from the input data).
> > taint(s[0][0]) == MEM_TO_SHADOW(s[0])[0] ====> taint(ret[0]) ==
> > MEM_TO_SHADOW(ret)[0]
> >
> > Because s[0] == ret (or ret==null), (for the non-null case) the output
> > shadow bytes are the same bytes as input shadow bytes and so these taint
> > labels for the string data in shadow memory do not need to be explicitly
> > propagated in this function.
> >
> > I think the only case actually changing/copying string data is writing a
> > delimiter byte to NULL, which you handled.
> I am working on the changes and I came across a strange behavior that I would
> like to ask about.
>
> It turned out that if we do
>
> char *s = " ... ";
> dfsan_set_label(label, &p_s, sizeof(&p_s));
>
> Then, the s_label within the handler is 0, not "label". This is unexpected,
> as we would like the pointer itself to be labeled here.
> I believe this has something to do with the fact that the input string in
> strsep is a double pointer. For example this works as expected for the
> delimiter string, which is a single pointer.
> It's either I'm doing something incorrectly or there is some issue with
> propagating labels for double pointers.
> Have you perhaps come across this behavior before?
I'm not sure what p_s is in your example. Could you provide a more complete
example?
(and maybe all in one function, not half in __dfsw_strsep and half in another
function)
Here is an example showing taint labels at different levels of indirection:
```
#include <assert.h>
#include <string.h>
#include <sanitizer/dfsan_interface.h>
int main(void) {
char *s = " ... ";
char **p_s = &s;
char ***pp_s = &p_s;
dfsan_label i_label = 1 << 0;
dfsan_label j_label = 1 << 1;
dfsan_label k_label = 1 << 2;
dfsan_label m_label = 1 << 3;
// string data
dfsan_set_label(i_label, s, strlen(s));
// pointer s
dfsan_set_label(j_label, &s, sizeof(s));
// pointer to pointer s
dfsan_set_label(k_label, &p_s, sizeof(p_s));
// pointer to pointer to pointer s
dfsan_set_label(m_label, &pp_s, sizeof(pp_s));
assert(pp_s[0][0] == s);
// string data
assert(dfsan_read_label(s, strlen(s)) == i_label);
// pointer s
assert(dfsan_read_label(&s, sizeof(s)) == j_label);
// pointer to pointer s
assert(dfsan_read_label(&p_s, sizeof(p_s)) == k_label);
// pointer to pointer to pointer s
assert(dfsan_read_label(&pp_s, sizeof(pp_s)) == m_label);
return 0;
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141389/new/
https://reviews.llvm.org/D141389
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
