[PATCH v2 RESEND] tracing: probeevent: Correctly update remaining space in dynamic area

2019-02-06 Thread Andreas Ziegler
Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.

This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.

Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic 
area")
Signed-off-by: Andreas Ziegler 
Acked-by: Masami Hiramatsu 
---
v2: follow coding guidelines for braces

 kernel/trace/trace_probe_tmpl.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..4737bb8c07a3 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, 
struct pt_regs *regs,
if (unlikely(arg->dynamic))
*dl = make_data_loc(maxlen, dyndata - base);
ret = process_fetch_insn(arg->code, regs, dl, base);
-   if (unlikely(ret < 0 && arg->dynamic))
+   if (unlikely(ret < 0 && arg->dynamic)) {
*dl = make_data_loc(0, dyndata - base);
-   else
+   } else {
dyndata += ret;
+   maxlen -= ret;
+   }
}
 }

--
2.17.1



[PATCH v2] tracing: probeevent: Correctly update remaining space in dynamic area

2019-01-22 Thread Andreas Ziegler
Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.

This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.

Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic 
area")
Signed-off-by: Andreas Ziegler 
Acked-by: Masami Hiramatsu 
---
v2: follow coding guidelines for braces

 kernel/trace/trace_probe_tmpl.h | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..4737bb8c07a3 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, 
struct pt_regs *regs,
if (unlikely(arg->dynamic))
*dl = make_data_loc(maxlen, dyndata - base);
ret = process_fetch_insn(arg->code, regs, dl, base);
-   if (unlikely(ret < 0 && arg->dynamic))
+   if (unlikely(ret < 0 && arg->dynamic)) {
*dl = make_data_loc(0, dyndata - base);
-   else
+   } else {
dyndata += ret;
+   maxlen -= ret;
+   }
}
 }

--
2.17.1



[PATCH] tracing: probeevent: Correctly update remaining space in dynamic area

2019-01-22 Thread Andreas Ziegler
Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.

This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.

Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic 
area")
Signed-off-by: Andreas Ziegler 
---
 kernel/trace/trace_probe_tmpl.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..0cf953e47584 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -182,8 +182,10 @@ store_trace_args(void *data, struct trace_probe *tp, 
struct pt_regs *regs,
ret = process_fetch_insn(arg->code, regs, dl, base);
if (unlikely(ret < 0 && arg->dynamic))
*dl = make_data_loc(0, dyndata - base);
-   else
+   else {
dyndata += ret;
+   maxlen -= ret;
+   }
}
 }
 
-- 
2.17.1



Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments

2019-01-17 Thread Andreas Ziegler
On 17. Jan 2019, at 15:51, Steven Rostedt  wrote:
> 
> On Thu, 17 Jan 2019 15:29:09 +0100
> Andreas Ziegler  wrote:
> 
>> From my side it's basically good to go, but I just realized that there 
>> is no "Fixes: " tag in it but the problem it fixes goes back to 
>> 5baaa59ef09e ("tracing/probes: Implement 'memory' fetch method for 
>> uprobes") from 2013 (and is present in all current longterm and stable 
>> kernels)... should I add a tag and resend a v3?
>> 
> 
> I was just about to ask you to provide the fixes tag. No need to
> resend, I'll just add it manually. I asked Masami to resend because it
> was a patch series. If it was just a single patch, I would have just
> asked what the fixes was for that particular patch. I can handle one
> patch, but not more than one ;-)
> 
> -- Steve

One more thing maybe, the code in question was rewritten
by Masami in 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area”) which is in v4.20 but the problem traces back
to the original commit mentioned above, so it will need manual so we
will need a slightly modified version for earlier kernels.

Regards,

Andreas

Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments

2019-01-17 Thread Andreas Ziegler

On 1/17/19 3:20 PM, Steven Rostedt wrote:

On Thu, 17 Jan 2019 16:58:07 +0900
Masami Hiramatsu  wrote:


Ah, I got it.
Hmm, in that case, I have to update my patch in the previous mail.
Anyway,

Acked-by: Masami Hiramatsu 


So this patch is good to go? If so, I'll pull it in and start testing
it.

I'm currently traveling (as you probably received my OoO emails), but I
can still do a bit of work remotely.

-- Steve

From my side it's basically good to go, but I just realized that there 
is no "Fixes: " tag in it but the problem it fixes goes back to 
5baaa59ef09e ("tracing/probes: Implement 'memory' fetch method for 
uprobes") from 2013 (and is present in all current longterm and stable 
kernels)... should I add a tag and resend a v3?


Thanks,

Andreas


Re: uprobes: bug in comm/string output?

2019-01-17 Thread Andreas Ziegler

Hi,

On 1/17/19 10:47 AM, Masami Hiramatsu wrote:

On Thu, 17 Jan 2019 09:08:41 +0100
Andreas Ziegler  wrote:


On 17.01.19 09:00, Masami Hiramatsu wrote:

On Thu, 17 Jan 2019 15:13:09 +0900
Masami Hiramatsu  wrote:


On Wed, 16 Jan 2019 11:16:07 +0100
Andreas Ziegler  wrote:



I went into this a bit deeper today, and right now it is simply failing
to parse the code because there is no FETCH_OP_COMM case in
process_fetch_insn() for uprobes so that will return -EILSEQ, leading to
a make_data_loc(0, ...) in store_trace_args(). If we just add
FETCH_OP_COMM and let val point to current->comm (that's what
trace_kprobe.c does), we get an -EFAULT return value from
fetch_store_string because strncpy_from_user() checks if the argument is
in user space.


Correct. I missed to add OP_COMM support. And uprobe's fetch_store_string
is only for user space strings.


So I think we might need a special case for that, something like
FETCH_OP_ST_COMM_STRING which is only used for FETCH_OP_COMM and copies
current->comm over to the dynamic area. The implementation could be
similar to the old fetch_comm_string implementation before your rewrite.


Hmm, instead, I would like to add current->comm checker and only allows
to copy that. That would be simpler and enough.

Could you test below patch?


tracing: uprobes: Re-enable $comm support for uprobe events

From: Masami Hiramatsu 

Since commit 533059281ee5 ("tracing: probeevent: Introduce new
argument fetching code") dropped the $comm support from uprobe
events, this re-enable it.


this should read 're-enables'.



For $comm support, use strncpy() instead of strncpy_from_user()

  ^
we're using strlcpy(), not strncpy().


to copy current task's comm. Because it is in the kernel space,
strncpy_from_user() always fails to copy the comm.
This also use strlen() instead of strlen_user() to measure the

^^
'uses', and the function should be 'strnlen_user()'.


length of the comm.

Signed-off-by: Masami Hiramatsu 
Reported-by: Andreas Ziegler 
---
  kernel/trace/trace_uprobe.c |   13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e335576b9411..97d134e83e0f 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -156,7 +156,10 @@ fetch_store_string(unsigned long addr, void *dest, void 
*base)
if (unlikely(!maxlen))
return -ENOMEM;
  
-	ret = strncpy_from_user(dst, src, maxlen);

+   if (addr == (unsigned long)current->comm)
+   ret = strlcpy(dst, current->comm, maxlen);
+   else
+   ret = strncpy_from_user(dst, src, maxlen);
if (ret >= 0) {
if (ret == maxlen)
dst[ret - 1] = '\0';
@@ -173,7 +176,10 @@ fetch_store_strlen(unsigned long addr)
int len;
void __user *vaddr = (void __force __user *) addr;
  
-	len = strnlen_user(vaddr, MAX_STRING_SIZE);

+   if (addr == (unsigned long)current->comm)
+   len = strlen(current->comm);


To balance with the strnlen_user, we must increse the len in this block.
(strlen doesn't count the final '\0', but strnlen_user counts it)



yes, we need to add a '+ 1' here.

With the typos and this one fixed, this is

Acked-by: Andreas Ziegler 


Thank you for fixing typo and Ack :)

Thanks you,

>



Thank you,


+   else
+   len = strnlen_user(vaddr, MAX_STRING_SIZE);
  
  	return (len > MAX_STRING_SIZE) ? 0 : len;

  }
@@ -213,6 +219,9 @@ process_fetch_insn(struct fetch_insn *code, struct pt_regs 
*regs, void *dest,
case FETCH_OP_IMM:
val = code->immediate;
break;
+   case FETCH_OP_COMM:
+   val = (unsigned long)current->comm;
+   break;
case FETCH_OP_FOFFS:
val = translate_user_vaddr(code->immediate);
break;








as the original commit breaking $comm support was merged for v4.20 
(which is a stable kernel) and the wrong behaviour with multiple strings 
exists in all longterm/stable releases (tested back to v4.4), do you 
think this should be going into a stable release once it's merged?


I added Greg as he might know the answer to that.

Thanks,

Andreas



smime.p7s
Description: S/MIME Cryptographic Signature


[PATCH] tracing: uprobes: fix typo in pr_fmt string

2019-01-17 Thread Andreas Ziegler
The subsystem-specific message prefix for uprobes was also
"trace_kprobe: " instead of "trace_uprobe: " as described in
the original commit message.

Fixes: 7257634135c24 ("tracing/probe: Show subsystem name in messages")
Signed-off-by: Andreas Ziegler 
---
 kernel/trace/trace_uprobe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e335576b9411..19a1a8e19062 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -5,7 +5,7 @@
  * Copyright (C) IBM Corporation, 2010-2012
  * Author: Srikar Dronamraju 
  */
-#define pr_fmt(fmt)"trace_kprobe: " fmt
+#define pr_fmt(fmt)"trace_uprobe: " fmt
 
 #include 
 #include 
-- 
2.17.1



Re: uprobes: bug in comm/string output?

2019-01-17 Thread Andreas Ziegler
On 17.01.19 09:00, Masami Hiramatsu wrote:
> On Thu, 17 Jan 2019 15:13:09 +0900
> Masami Hiramatsu  wrote:
> 
>> On Wed, 16 Jan 2019 11:16:07 +0100
>> Andreas Ziegler  wrote:
>>
>>>
>>> I went into this a bit deeper today, and right now it is simply failing 
>>> to parse the code because there is no FETCH_OP_COMM case in 
>>> process_fetch_insn() for uprobes so that will return -EILSEQ, leading to 
>>> a make_data_loc(0, ...) in store_trace_args(). If we just add 
>>> FETCH_OP_COMM and let val point to current->comm (that's what 
>>> trace_kprobe.c does), we get an -EFAULT return value from 
>>> fetch_store_string because strncpy_from_user() checks if the argument is 
>>> in user space.
>>
>> Correct. I missed to add OP_COMM support. And uprobe's fetch_store_string
>> is only for user space strings.
>>
>>> So I think we might need a special case for that, something like 
>>> FETCH_OP_ST_COMM_STRING which is only used for FETCH_OP_COMM and copies 
>>> current->comm over to the dynamic area. The implementation could be 
>>> similar to the old fetch_comm_string implementation before your rewrite.
>>
>> Hmm, instead, I would like to add current->comm checker and only allows
>> to copy that. That would be simpler and enough.
>>
>> Could you test below patch?
>>
>>
>> tracing: uprobes: Re-enable $comm support for uprobe events
>>
>> From: Masami Hiramatsu 
>>
>> Since commit 533059281ee5 ("tracing: probeevent: Introduce new
>> argument fetching code") dropped the $comm support from uprobe
>> events, this re-enable it.

this should read 're-enables'.

>>
>> For $comm support, use strncpy() instead of strncpy_from_user()
 ^
we're using strlcpy(), not strncpy().

>> to copy current task's comm. Because it is in the kernel space,
>> strncpy_from_user() always fails to copy the comm.
>> This also use strlen() instead of strlen_user() to measure the
   ^^
'uses', and the function should be 'strnlen_user()'.

>> length of the comm.
>>
>> Signed-off-by: Masami Hiramatsu 
>> Reported-by: Andreas Ziegler 
>> ---
>>  kernel/trace/trace_uprobe.c |   13 +++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index e335576b9411..97d134e83e0f 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -156,7 +156,10 @@ fetch_store_string(unsigned long addr, void *dest, void 
>> *base)
>>  if (unlikely(!maxlen))
>>  return -ENOMEM;
>>  
>> -ret = strncpy_from_user(dst, src, maxlen);
>> +if (addr == (unsigned long)current->comm)
>> +ret = strlcpy(dst, current->comm, maxlen);
>> +else
>> +ret = strncpy_from_user(dst, src, maxlen);
>>  if (ret >= 0) {
>>  if (ret == maxlen)
>>  dst[ret - 1] = '\0';
>> @@ -173,7 +176,10 @@ fetch_store_strlen(unsigned long addr)
>>  int len;
>>  void __user *vaddr = (void __force __user *) addr;
>>  
>> -len = strnlen_user(vaddr, MAX_STRING_SIZE);
>> +if (addr == (unsigned long)current->comm)
>> +len = strlen(current->comm);
> 
> To balance with the strnlen_user, we must increse the len in this block.
> (strlen doesn't count the final '\0', but strnlen_user counts it)
> 

yes, we need to add a '+ 1' here.

With the typos and this one fixed, this is

Acked-by: Andreas Ziegler 

> Thank you,
> 
>> +else
>> +len = strnlen_user(vaddr, MAX_STRING_SIZE);
>>  
>>  return (len > MAX_STRING_SIZE) ? 0 : len;
>>  }
>> @@ -213,6 +219,9 @@ process_fetch_insn(struct fetch_insn *code, struct 
>> pt_regs *regs, void *dest,
>>  case FETCH_OP_IMM:
>>  val = code->immediate;
>>  break;
>> +case FETCH_OP_COMM:
>> +val = (unsigned long)current->comm;
>> +break;
>>  case FETCH_OP_FOFFS:
>>  val = translate_user_vaddr(code->immediate);
>>  break;
> 
> 



Re: [PATCH v2] tracing/uprobes: Fix output for multiple string arguments

2019-01-16 Thread Andreas Ziegler
On 17.01.19 07:01, Masami Hiramatsu wrote:
> On Wed, 16 Jan 2019 15:16:29 +0100
> Andreas Ziegler  wrote:
> 
>> When printing multiple uprobe arguments as strings the output for the
>> earlier arguments would also include all later string arguments.
>>
>> This is best explained in an example:
>>
>> Consider adding a uprobe to a function receiving two strings as
>> parameters which is at offset 0xa0 in strlib.so and we want to print
>> both parameters when the uprobe is hit (on x86_64):
>>
>> $ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \
>> /sys/kernel/debug/tracing/uprobe_events
>>
>> When the function is called as func("foo", "bar") and we hit the probe,
>> the trace file shows a line like the following:
>>
>>   [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar"
>>
>> Note the extra "bar" printed as part of arg1. This behaviour stacks up
>> for additional string arguments.
>>
>> The strings are stored in a dynamically growing part of the uprobe
>> buffer by fetch_store_string() after copying them from userspace via
>> strncpy_from_user(). The return value of strncpy_from_user() is then
>> directly used as the required size for the string. However, this does
>> not take the terminating null byte into account as the documentation
>> for strncpy_from_user() cleary states that it "[...] returns the
>> length of the string (not including the trailing NUL)" even though the
>> null byte will be copied to the destination.
>>
>> Therefore, subsequent calls to fetch_store_string() will overwrite
>> the terminating null byte of the most recently fetched string with
>> the first character of the current string, leading to the
>> "accumulation" of strings in earlier arguments in the output.
>>
>> Fix this by incrementing the return value of strncpy_from_user() by
>> one if we did not hit the maximum buffer size.
>>
> 
> Yeah, I had eventually same conclusion. However, you also have to increse
> the return value of fetch_store_strlen() too. (And I found another issue)
> 

I don't think we need to increase that since the documentation for
strnlen_user() says that it "[r]eturns the size of the string
INCLUDING the terminating NUL." so its return value will already be
one more than that of strncpy_from_user().

Thanks,

Andreas

> Could you fix fetch_store_strlen in the same patch?
> 
> Thank you,
> 
>> Signed-off-by: Andreas Ziegler 
>> ---
>> v2: removed a wrong check for (ret > 0)
>>
>>  kernel/trace/trace_uprobe.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index e335576b9411..3a1d5ab6b4ba 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void 
>> *base)
>>  if (ret >= 0) {
>>  if (ret == maxlen)
>>  dst[ret - 1] = '\0';
>> +else
>> +/*
>> + * Include the terminating null byte. In this case it
>> + * was copied by strncpy_from_user but not accounted
>> + * for in ret.
>> + */
>> +ret++;
>>  *(u32 *)dest = make_data_loc(ret, (void *)dst - base);
>>  }
>>
>> --
>> 2.17.1
>>
> 
> 



[PATCH v2] tracing/uprobes: Fix output for multiple string arguments

2019-01-16 Thread Andreas Ziegler
When printing multiple uprobe arguments as strings the output for the
earlier arguments would also include all later string arguments.

This is best explained in an example:

Consider adding a uprobe to a function receiving two strings as
parameters which is at offset 0xa0 in strlib.so and we want to print
both parameters when the uprobe is hit (on x86_64):

$ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \
/sys/kernel/debug/tracing/uprobe_events

When the function is called as func("foo", "bar") and we hit the probe,
the trace file shows a line like the following:

  [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar"

Note the extra "bar" printed as part of arg1. This behaviour stacks up
for additional string arguments.

The strings are stored in a dynamically growing part of the uprobe
buffer by fetch_store_string() after copying them from userspace via
strncpy_from_user(). The return value of strncpy_from_user() is then
directly used as the required size for the string. However, this does
not take the terminating null byte into account as the documentation
for strncpy_from_user() cleary states that it "[...] returns the
length of the string (not including the trailing NUL)" even though the
null byte will be copied to the destination.

Therefore, subsequent calls to fetch_store_string() will overwrite
the terminating null byte of the most recently fetched string with
the first character of the current string, leading to the
"accumulation" of strings in earlier arguments in the output.

Fix this by incrementing the return value of strncpy_from_user() by
one if we did not hit the maximum buffer size.

Signed-off-by: Andreas Ziegler 
---
v2: removed a wrong check for (ret > 0)

 kernel/trace/trace_uprobe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e335576b9411..3a1d5ab6b4ba 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void 
*base)
if (ret >= 0) {
if (ret == maxlen)
dst[ret - 1] = '\0';
+   else
+   /*
+* Include the terminating null byte. In this case it
+* was copied by strncpy_from_user but not accounted
+* for in ret.
+*/
+   ret++;
*(u32 *)dest = make_data_loc(ret, (void *)dst - base);
}

--
2.17.1



Re: [PATCH] tracing/uprobes: Fix output for multiple string arguments

2019-01-16 Thread Andreas Ziegler

On 1/16/19 2:40 PM, Steven Rostedt wrote:

On Wed, 16 Jan 2019 10:41:12 +0100
Andreas Ziegler  wrote:


When printing multiple uprobe arguments as strings the output for the
earlier arguments would also include all later string arguments.

This is best explained in an example:

Consider adding a uprobe to a function receiving two strings as
parameters which is at offset 0xa0 in strlib.so and we want to print
both parameters when the uprobe is hit (on x86_64):

$ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \
 /sys/kernel/debug/tracing/uprobe_events

When the function is called as func("foo", "bar") and we hit the probe,
the trace file shows a line like the following:

   [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar"

Note the extra "bar" printed as part of arg1. This behaviour stacks up
for additional string arguments.

The strings are stored in a dynamically growing part of the uprobe
buffer by fetch_store_string() after copying them from userspace via
strncpy_from_user(). The return value of strncpy_from_user() is then
directly used as the required size for the string. However, this does
not take the terminating null byte into account as the documentation
for strncpy_from_user() cleary states that it "[...] returns the
length of the string (not including the trailing NUL)" even though the
null byte will be copied to the destination.

Therefore, subsequent calls to fetch_store_string() will overwrite
the terminating null byte of the most recently fetched string with
the first character of the current string, leading to the
"accumulation" of strings in earlier arguments in the output.

Fix this by incrementing the return value of strncpy_from_user() by
one if we did not hit the maximum buffer size.

Signed-off-by: Andreas Ziegler 
---
  kernel/trace/trace_uprobe.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e335576b9411..dfb9bbc7fd82 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void 
*base)
if (ret >= 0) {
if (ret == maxlen)
dst[ret - 1] = '\0';
+   else if (ret > 0)


Do we need the ret > 0 check? What if the value is ""?

Doesn't that cause the same issue?

-- Steve



yes, it does. With this patch an empty string will also print "(fault)", 
I missed that, sorry. I'll send a v2.


Thanks,

Andreas


+   /*
+* Include the terminating null byte. In this case it
+* was copied by strncpy_from_user but not accounted
+* for in ret.
+*/
+   ret++;
*(u32 *)dest = make_data_loc(ret, (void *)dst - base);
}
  






smime.p7s
Description: S/MIME Cryptographic Signature


Re: uprobes: bug in comm/string output?

2019-01-16 Thread Andreas Ziegler

Hi,

thanks for your reply!

On 1/16/19 11:00 AM, Masami Hiramatsu wrote:

On Tue, 15 Jan 2019 14:36:48 +0100
Andreas Ziegler  wrote:


Hi again,

On 1/14/19 1:38 PM, Andreas Ziegler wrote:

Hi,

I've been playing around with uprobes today and found the following weird 
behaviour/output when using more than one string argument (or using the $comm 
argument). This was run on a v4.20 mainline build on Ubuntu 18.04.

root@ubuntu1810:~# uname -a
Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 
UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

I'm trying to track calls to dlopen so I'm looking up the correct offset in 
libdl.so:

root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s 
/lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen
 34: 12a0   133 FUNCGLOBAL DEFAULT   14 dlopen@@GLIBC_2.2.5

Then I'm creating a uprobe with two prints of $comm and two prints of the first argument 
to dlopen, and enable that probe. The '/root/test' program only does a 
dlopen("libc.so.6", RTLD_LAZY) in main().

root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen 
/lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string 
+0(%di):string' > uprobe_events
root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable
root@ubuntu1810:/sys/kernel/debug/tracing# /root/test

The trace output looks like this:

root@ubuntu1810:/sys/kernel/debug/tracing# cat trace
# tracer: nop
#
#  _-=> irqs-off
# / _=> need-resched
#| / _---=> hardirq/softirq
#|| / _--=> preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
 test-1617  [000] d...  1237.959168: dlopen: (0x7fbd5272e2a0) arg1=(fault) arg2=(fault) 
arg3="libc.so.6libc.so.6" arg4="libc.so.6"

That's very weird for two reasons:
- fetching $comm seems to fail with an invalid pointer
- arg3 contains the text twice (if I add another print of the argument, arg3 
will contain the wanted string three times, arg4 two times and the last 
argument will contain it once).


at least for the second problem I think I found the answer, and for the
first problem I have a suspicion (see last paragraph for that).


OK, this looks broken. Thank you very much for reporting it!

BTW, I tried to reproduce it with kprobe event, but it seems working well. e.g.

  # echo 'p ksys_chdir $comm $comm +0(%di):string +0(%di):string' > 
kprobe_events
  # echo 1 > events/kprobes/enable
  # cd /sys/kernel/debug/tracing
  # cat trace
   sh-812   [003] ...1   229.344360: p_ksys_chdir_0: (ksys_chdir+0x0/0xc0) arg1="sh" 
arg2="sh" arg3="/sys/kernel/debug/tracing" arg4="/sys/kernel/debug/tracing"

So, it might be an issue on uprobe_event.



yes, kprobes work because they use strncpy_from_unsafe which *includes* 
the null byte in its return value... the fact that both are called 
strncpy_* but behave differently is really annoying...


I just sent a patch for this case half an hour ago which simply adds 1 
to the returned value for uprobes if it didn't hit the maximum allowed 
length.




For this, I installed a uprobe for libdl.so/dlopen once again, the
command would be

'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm'

so it should print the process name twice (using a kernel v4.18 on
Ubuntu 18.10).

The code which prints the collected data (print_type_string, defined by
PRINT_TYPE_FUNC_NAME(string) in kernel/trace/trace_probe.c) is the
following, it simply passes the respective data to trace_seq_printf with
a "%s" format string:

int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s, void *data, void *ent)
{
  int len = *(u32 *)data >> 16;

  if (!len)
  trace_seq_puts(s, "(fault)");
  else
  trace_seq_printf(s, "\"%s\"",
   (const char *)get_loc_data(data, ent));
  return !trace_seq_has_overflowed(s);
}

I dug into that function with KGDB and found the following: 'data' holds
the size and offset for the member in question, which is 0xf and 0x18,
respectively. 'ent' holds the base address for event. When we print the
string at ent + 0x18, we can see that the output for 'arg1' will be
"update-notifierupdate-notifier"

Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090,
name=0x880078fe4458 "arg1", data=0x88007d01f05c,
ent=0x88007d01f04c) at
/build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67
67  in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c
gdb$ p *(uint32_t *) data
$46 = 0xf0018
gdb$ p ent
$47 = (void *) 0x88007d01f04c
gdb$ p ((char *)ent + 0x18)
$48 = 0x88

[PATCH] tracing/uprobes: Fix output for multiple string arguments

2019-01-16 Thread Andreas Ziegler
When printing multiple uprobe arguments as strings the output for the
earlier arguments would also include all later string arguments.

This is best explained in an example:

Consider adding a uprobe to a function receiving two strings as
parameters which is at offset 0xa0 in strlib.so and we want to print
both parameters when the uprobe is hit (on x86_64):

$ echo 'p:func /lib/strlib.so:0xa0 +0(%di):string +0(%si):string' > \
/sys/kernel/debug/tracing/uprobe_events

When the function is called as func("foo", "bar") and we hit the probe,
the trace file shows a line like the following:

  [...] func: (0x7f7e683706a0) arg1="foobar" arg2="bar"

Note the extra "bar" printed as part of arg1. This behaviour stacks up
for additional string arguments.

The strings are stored in a dynamically growing part of the uprobe
buffer by fetch_store_string() after copying them from userspace via
strncpy_from_user(). The return value of strncpy_from_user() is then
directly used as the required size for the string. However, this does
not take the terminating null byte into account as the documentation
for strncpy_from_user() cleary states that it "[...] returns the
length of the string (not including the trailing NUL)" even though the
null byte will be copied to the destination.

Therefore, subsequent calls to fetch_store_string() will overwrite
the terminating null byte of the most recently fetched string with
the first character of the current string, leading to the
"accumulation" of strings in earlier arguments in the output.

Fix this by incrementing the return value of strncpy_from_user() by
one if we did not hit the maximum buffer size.

Signed-off-by: Andreas Ziegler 
---
 kernel/trace/trace_uprobe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index e335576b9411..dfb9bbc7fd82 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -160,6 +160,13 @@ fetch_store_string(unsigned long addr, void *dest, void 
*base)
if (ret >= 0) {
if (ret == maxlen)
dst[ret - 1] = '\0';
+   else if (ret > 0)
+   /*
+* Include the terminating null byte. In this case it
+* was copied by strncpy_from_user but not accounted
+* for in ret.
+*/
+   ret++;
*(u32 *)dest = make_data_loc(ret, (void *)dst - base);
}
 
-- 
2.17.1



Re: uprobes: bug in comm/string output?

2019-01-15 Thread Andreas Ziegler

Hi again,

On 1/14/19 1:38 PM, Andreas Ziegler wrote:

Hi,

I've been playing around with uprobes today and found the following weird 
behaviour/output when using more than one string argument (or using the $comm 
argument). This was run on a v4.20 mainline build on Ubuntu 18.04.

root@ubuntu1810:~# uname -a
Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 
UTC 2018 x86_64 x86_64 x86_64 GNU/Linux

I'm trying to track calls to dlopen so I'm looking up the correct offset in 
libdl.so:

root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s 
/lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen
34: 12a0   133 FUNCGLOBAL DEFAULT   14 dlopen@@GLIBC_2.2.5

Then I'm creating a uprobe with two prints of $comm and two prints of the first argument 
to dlopen, and enable that probe. The '/root/test' program only does a 
dlopen("libc.so.6", RTLD_LAZY) in main().

root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen 
/lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string 
+0(%di):string' > uprobe_events
root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable
root@ubuntu1810:/sys/kernel/debug/tracing# /root/test

The trace output looks like this:

root@ubuntu1810:/sys/kernel/debug/tracing# cat trace
# tracer: nop
#
#  _-=> irqs-off
# / _=> need-resched
#| / _---=> hardirq/softirq
#|| / _--=> preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
test-1617  [000] d...  1237.959168: dlopen: (0x7fbd5272e2a0) arg1=(fault) arg2=(fault) arg3="libc.so.6libc.so.6" arg4="libc.so.6" 


That's very weird for two reasons:
- fetching $comm seems to fail with an invalid pointer
- arg3 contains the text twice (if I add another print of the argument, arg3 will contain the wanted string three times, arg4 two times and the last argument will contain it once). 


at least for the second problem I think I found the answer, and for the 
first problem I have a suspicion (see last paragraph for that).


For this, I installed a uprobe for libdl.so/dlopen once again, the 
command would be


'p:dlopen /lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm'

so it should print the process name twice (using a kernel v4.18 on 
Ubuntu 18.10).


The code which prints the collected data (print_type_string, defined by 
PRINT_TYPE_FUNC_NAME(string) in kernel/trace/trace_probe.c) is the 
following, it simply passes the respective data to trace_seq_printf with 
a "%s" format string:


int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s, void *data, void *ent)
{
int len = *(u32 *)data >> 16;

if (!len)
trace_seq_puts(s, "(fault)");
else
trace_seq_printf(s, "\"%s\"",
 (const char *)get_loc_data(data, ent));
return !trace_seq_has_overflowed(s);
}

I dug into that function with KGDB and found the following: 'data' holds 
the size and offset for the member in question, which is 0xf and 0x18, 
respectively. 'ent' holds the base address for event. When we print the 
string at ent + 0x18, we can see that the output for 'arg1' will be 
"update-notifierupdate-notifier"


Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090, 
name=0x880078fe4458 "arg1", data=0x88007d01f05c, 
ent=0x88007d01f04c) at 
/build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67

67  in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c
gdb$ p *(uint32_t *) data
$46 = 0xf0018
gdb$ p ent
$47 = (void *) 0x88007d01f04c
gdb$ p ((char *)ent + 0x18)
$48 = 0x88007d01f064 "update-notifierupdate-notifier"

Moving on printing 'arg2' (e.g., the other $comm string). Here we see 
that the string in question is at offset 0x27, and if we print that, we 
see the correct "update-notifier".


Thread 511 hit Breakpoint 6, print_type_string (s=0x880078fd1090, 
name=0x880078fe4d70 "arg2", data=0x88007d01f060, 
ent=0x88007d01f04c) at 
/build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c:67

67  in /build/linux-EsXT4r/linux-4.18.0/kernel/trace/trace_probe.c
gdb$ p *(uint32_t *) data
$49 = 0xf0027
gdb$ p ((char *)ent + 0x27)
$50 = 0x88007d01f073 "update-notifier"

Looking at the bytes in memory and the offsets it becomes clear that 
there is no \0 byte at the end of the first entry (which would need to 
be at address 0x88007d01f064 + 0xf = 0x88007d01f073 but instead 
that's the start address of the second entry which simply gets consumed 
by the (first) "%s" as well.


gdb$ x/32x ent
0x88007d01f04c: 0x00010592  0x2143

uprobes: bug in comm/string output?

2019-01-14 Thread Andreas Ziegler

Hi,

I've been playing around with uprobes today and found the following weird 
behaviour/output when using more than one string argument (or using the $comm 
argument). This was run on a v4.20 mainline build on Ubuntu 18.04.


root@ubuntu1810:~# uname -a
Linux ubuntu1810 4.20.0-042000-generic #201812232030 SMP Mon Dec 24 01:32:58 UTC 
2018 x86_64 x86_64 x86_64 GNU/Linux


I'm trying to track calls to dlopen so I'm looking up the correct offset in 
libdl.so:


root@ubuntu1810:/sys/kernel/debug/tracing# readelf -s 
/lib/x86_64-linux-gnu/libdl-2.28.so | grep dlopen

34: 12a0   133 FUNCGLOBAL DEFAULT   14 dlopen@@GLIBC_2.2.5

Then I'm creating a uprobe with two prints of $comm and two prints of the first 
argument to dlopen, and enable that probe. The '/root/test' program only does a 
dlopen("libc.so.6", RTLD_LAZY) in main().


root@ubuntu1810:/sys/kernel/debug/tracing# echo 'p:dlopen 
/lib/x86_64-linux-gnu/libdl-2.28.so:0x12a0 $comm $comm +0(%di):string 
+0(%di):string' > uprobe_events

root@ubuntu1810:/sys/kernel/debug/tracing# echo 1 > events/uprobes/dlopen/enable
root@ubuntu1810:/sys/kernel/debug/tracing# /root/test

The trace output looks like this:

root@ubuntu1810:/sys/kernel/debug/tracing# cat trace
# tracer: nop
#
#  _-=> irqs-off
# / _=> need-resched
#| / _---=> hardirq/softirq
#|| / _--=> preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
test-1617  [000] d...  1237.959168: dlopen: (0x7fbd5272e2a0) 
arg1=(fault) arg2=(fault) arg3="libc.so.6libc.so.6" arg4="libc.so.6"


That's very weird for two reasons:
- fetching $comm seems to fail with an invalid pointer
- arg3 contains the text twice (if I add another print of the argument, arg3 
will contain the wanted string three times, arg4 two times and the last argument 
will contain it once).


On the "standard" kernel shipped with Ubuntu 18.04 (4.18) printing $comm works 
but also "accumulates" (in lack of a better word) the later string arguments in 
the earlier output arguments, so for the probe above arg1 would be 
"testtestlibc.so.6libc.so.6", arg2 would be "testlibc.so.6libc.so.6" and 
arg3/arg4 will be the same as in the output above.


Is there anything in the documentation why multiple string outputs might not 
work with uprobes? Am I installing the uprobe wrong? Or is this a bug?


I found that the kprobe selftest at 
tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc contains a 
similar situation for kprobes (it prints the parameter two times) which works 
fine on the same system.


Regards,

Andreas



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH] regmap: make LZO cache optional

2017-06-08 Thread Andreas Ziegler
Hi Jonas,

I noticed your patch 'regmap: make LZO cache optional' as it recently showed up
in linux-next. In your patch, you modify drivers/base/regmap/regcache.c by
adding an #if IS_ENABLED() statement.

However, this statement contains a spelling error, as it references
REGCHACHE_COMPRESSED instead of REGCACHE_COMPRESSED (note the extra H).

I noticed it by running the in-tree script at scripts/checkkconfigsymbols.py on
the commit, like so: './scripts/checkkconfigsymbols.py -c 34a730aa74c7'

As Greg suggested the whole code could be dropped, this might not be too
relevant, but I wanted to let you know in any case.

Best regards,

Andreas



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH] regmap: make LZO cache optional

2017-06-08 Thread Andreas Ziegler
Hi Jonas,

I noticed your patch 'regmap: make LZO cache optional' as it recently showed up
in linux-next. In your patch, you modify drivers/base/regmap/regcache.c by
adding an #if IS_ENABLED() statement.

However, this statement contains a spelling error, as it references
REGCHACHE_COMPRESSED instead of REGCACHE_COMPRESSED (note the extra H).

I noticed it by running the in-tree script at scripts/checkkconfigsymbols.py on
the commit, like so: './scripts/checkkconfigsymbols.py -c 34a730aa74c7'

As Greg suggested the whole code could be dropped, this might not be too
relevant, but I wanted to let you know in any case.

Best regards,

Andreas



smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH v3] cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB SoCs

2016-11-28 Thread Andreas Ziegler
Forgot to CC linux-kernel and linux-pm in my first message.

On 11/28/2016 10:28, Andreas Ziegler wrote:
> Hi Markus,
> 
> your patch "cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB
> SoCs" showed up in linux-next today, and I noticed a small error in it.
> 
> In drivers/cpufreq/brcmstb-cpufreq.c, line 214 there is an IS_ENABLED()
> statement with CONFIG_ARM_BRCM_AVS_CPUFREQ. This symbol, however, does not 
> exist
> - making the if condition always evaluate to false. Did you mean to use
> CONFIG_ARM_BRCMSTB_AVS_CPUFREQ (note the STB after BRCM)?
> 
> I noticed this mistake by running 'scripts/checkkconfigsymbols -f --force -f
> next-20161124..next-20161128', which essentialy diffs the last two
> linux-next releases and looks for undefined/unknown Kconfig symbols.
> You can also run the script on single commits with -c to test them.
> 
> Best regards,
> 
> Andreas
> 


Re: [PATCH v3] cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB SoCs

2016-11-28 Thread Andreas Ziegler
Forgot to CC linux-kernel and linux-pm in my first message.

On 11/28/2016 10:28, Andreas Ziegler wrote:
> Hi Markus,
> 
> your patch "cpufreq: brcmstb-cpufreq: CPUfreq driver for older Broadcom STB
> SoCs" showed up in linux-next today, and I noticed a small error in it.
> 
> In drivers/cpufreq/brcmstb-cpufreq.c, line 214 there is an IS_ENABLED()
> statement with CONFIG_ARM_BRCM_AVS_CPUFREQ. This symbol, however, does not 
> exist
> - making the if condition always evaluate to false. Did you mean to use
> CONFIG_ARM_BRCMSTB_AVS_CPUFREQ (note the STB after BRCM)?
> 
> I noticed this mistake by running 'scripts/checkkconfigsymbols -f --force -f
> next-20161124..next-20161128', which essentialy diffs the last two
> linux-next releases and looks for undefined/unknown Kconfig symbols.
> You can also run the script on single commits with -c to test them.
> 
> Best regards,
> 
> Andreas
> 


Re: [PATCH v3] module: extend 'rodata=off' boot cmdline parameter to module mappings

2016-11-28 Thread Andreas Ziegler
Hi Akashi,

your patch "module: extend 'rodata=off' boot cmdline parameter to module
mappings" showed up in linux-next today, and I noticed a small error in it.

The first modified #ifdef is fine, the second one, however, has a spelling
mistake in it: the CONFIG_ variable should be CONFIG_DEBUG_SET_MODULE_RONX
instead of CONFIG_SET_MODULE_RONX (note the missing DEBUG).

I noticed it by running 'scripts/checkkconfigsymbols -f --force -f
next-20161124..next-20161128', which is essentialy diffing the last two
linux-next releases and looks for undefined/unknown Kconfig symbols.
You can also run the script on single commits with -c to test them.

Best regards,

Andreas


Re: [PATCH v3] module: extend 'rodata=off' boot cmdline parameter to module mappings

2016-11-28 Thread Andreas Ziegler
Hi Akashi,

your patch "module: extend 'rodata=off' boot cmdline parameter to module
mappings" showed up in linux-next today, and I noticed a small error in it.

The first modified #ifdef is fine, the second one, however, has a spelling
mistake in it: the CONFIG_ variable should be CONFIG_DEBUG_SET_MODULE_RONX
instead of CONFIG_SET_MODULE_RONX (note the missing DEBUG).

I noticed it by running 'scripts/checkkconfigsymbols -f --force -f
next-20161124..next-20161128', which is essentialy diffing the last two
linux-next releases and looks for undefined/unknown Kconfig symbols.
You can also run the script on single commits with -c to test them.

Best regards,

Andreas


[PATCH] printk: Remove unnecessary #ifdef CONFIG_PRINTK

2016-08-04 Thread Andreas Ziegler
In commit 874f9c7da9a4 ("printk: create pr_ functions") in
linux-next, new pr_level defines were added to printk.c. These
defines are guarded by an #ifdef CONFIG_PRINTK - however, there is
already a surrounding #ifdef CONFIG_PRINTK starting a lot earlier
in line 249 which means the newly introduced #ifdef is unnecessary.

Let's remove it to avoid confusion.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 kernel/printk/printk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ef95c..a37fc8c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1930,7 +1930,6 @@ asmlinkage int printk_emit(int facility, int level,
 }
 EXPORT_SYMBOL(printk_emit);
 
-#ifdef CONFIG_PRINTK
 #define define_pr_level(func, loglevel)\
 asmlinkage __visible void func(const char *fmt, ...)   \
 {  \
@@ -1949,7 +1948,6 @@ define_pr_level(__pr_err, LOGLEVEL_ERR);
 define_pr_level(__pr_warn, LOGLEVEL_WARNING);
 define_pr_level(__pr_notice, LOGLEVEL_NOTICE);
 define_pr_level(__pr_info, LOGLEVEL_INFO);
-#endif
 
 int vprintk_default(int level, const char *fmt, va_list args)
 {
-- 
1.9.1



[PATCH] printk: Remove unnecessary #ifdef CONFIG_PRINTK

2016-08-04 Thread Andreas Ziegler
In commit 874f9c7da9a4 ("printk: create pr_ functions") in
linux-next, new pr_level defines were added to printk.c. These
defines are guarded by an #ifdef CONFIG_PRINTK - however, there is
already a surrounding #ifdef CONFIG_PRINTK starting a lot earlier
in line 249 which means the newly introduced #ifdef is unnecessary.

Let's remove it to avoid confusion.

Signed-off-by: Andreas Ziegler 
---
 kernel/printk/printk.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a5ef95c..a37fc8c 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1930,7 +1930,6 @@ asmlinkage int printk_emit(int facility, int level,
 }
 EXPORT_SYMBOL(printk_emit);
 
-#ifdef CONFIG_PRINTK
 #define define_pr_level(func, loglevel)\
 asmlinkage __visible void func(const char *fmt, ...)   \
 {  \
@@ -1949,7 +1948,6 @@ define_pr_level(__pr_err, LOGLEVEL_ERR);
 define_pr_level(__pr_warn, LOGLEVEL_WARNING);
 define_pr_level(__pr_notice, LOGLEVEL_NOTICE);
 define_pr_level(__pr_info, LOGLEVEL_INFO);
-#endif
 
 int vprintk_default(int level, const char *fmt, va_list args)
 {
-- 
1.9.1



Re: [PATCHv2 0/7] eBPF JIT for PPC64

2016-06-30 Thread Andreas Ziegler
Hi Naveen,

this patchset makes a change to arch/powerpc/net/Makefile in order to only
compile the previously existing bpf_jit_comp.c if !CONFIG_PPC64, and use
bpf_jit_comp64.c if CONFIG_PPC64 is enabled.

Inside arch/powerpc/net/bpf_jit_comp.c, however, there is still an #ifdef
CONFIG_PPC64 block at line 667 (linux-next of today, i.e., next-20160630):

  #ifdef CONFIG_PPC64
  /* Function descriptor nastiness: Address + TOC */
  ((u64 *)image)[0] = (u64)code_base;
  ((u64 *)image)[1] = local_paca->kernel_toc;
  #endif

>From my understanding of the code, this #ifdef can now be removed, as there is
no way the file could be compiled with CONFIG_PPC64 enabled. Is this correct?

Best regards,

Andreas


Re: [PATCHv2 0/7] eBPF JIT for PPC64

2016-06-30 Thread Andreas Ziegler
Hi Naveen,

this patchset makes a change to arch/powerpc/net/Makefile in order to only
compile the previously existing bpf_jit_comp.c if !CONFIG_PPC64, and use
bpf_jit_comp64.c if CONFIG_PPC64 is enabled.

Inside arch/powerpc/net/bpf_jit_comp.c, however, there is still an #ifdef
CONFIG_PPC64 block at line 667 (linux-next of today, i.e., next-20160630):

  #ifdef CONFIG_PPC64
  /* Function descriptor nastiness: Address + TOC */
  ((u64 *)image)[0] = (u64)code_base;
  ((u64 *)image)[1] = local_paca->kernel_toc;
  #endif

>From my understanding of the code, this #ifdef can now be removed, as there is
no way the file could be compiled with CONFIG_PPC64 enabled. Is this correct?

Best regards,

Andreas


select on non-existing Kconfig option CRC32C

2016-06-15 Thread Andreas Ziegler
Hi Hendrik,

your patch "s390/crc32-vx: add crypto API module for optimized CRC-32
algorithms" showed up in linux-next today (next-20160615) as commit 
364148e0b195.

The patch defines the Kconfig option CRYPTO_CRC32_S390 which 'select's CRC32C.
However, this should probably have been CRYPTO_CRC32C, as CRC32C does not exist.
Should I prepare a trivial patch to fix this up or would you like to do that on
your side?

I found this issue by comparing yesterday's tree and today's tree using
'scripts/checkkconfigsymbols -f -d next-20160614..next-20160615'.

Best regards,

Andreas


select on non-existing Kconfig option CRC32C

2016-06-15 Thread Andreas Ziegler
Hi Hendrik,

your patch "s390/crc32-vx: add crypto API module for optimized CRC-32
algorithms" showed up in linux-next today (next-20160615) as commit 
364148e0b195.

The patch defines the Kconfig option CRYPTO_CRC32_S390 which 'select's CRC32C.
However, this should probably have been CRYPTO_CRC32C, as CRC32C does not exist.
Should I prepare a trivial patch to fix this up or would you like to do that on
your side?

I found this issue by comparing yesterday's tree and today's tree using
'scripts/checkkconfigsymbols -f -d next-20160614..next-20160615'.

Best regards,

Andreas


[PATCH] drivers/net/fsl_ucc: Do not prefix header guard with CONFIG_

2016-06-08 Thread Andreas Ziegler
The CONFIG_ prefix should only be used for options which
can be configured through Kconfig and not for guarding headers.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 drivers/net/wan/fsl_ucc_hdlc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index 525786a..881ecde 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -8,8 +8,8 @@
  * option) any later version.
  */
 
-#ifndef CONFIG_UCC_HDLC_H
-#define CONFIG_UCC_HDLC_H
+#ifndef _UCC_HDLC_H_
+#define _UCC_HDLC_H_
 
 #include 
 #include 
-- 
1.9.1



[PATCH] drivers/net/fsl_ucc: Do not prefix header guard with CONFIG_

2016-06-08 Thread Andreas Ziegler
The CONFIG_ prefix should only be used for options which
can be configured through Kconfig and not for guarding headers.

Signed-off-by: Andreas Ziegler 
---
 drivers/net/wan/fsl_ucc_hdlc.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index 525786a..881ecde 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -8,8 +8,8 @@
  * option) any later version.
  */
 
-#ifndef CONFIG_UCC_HDLC_H
-#define CONFIG_UCC_HDLC_H
+#ifndef _UCC_HDLC_H_
+#define _UCC_HDLC_H_
 
 #include 
 #include 
-- 
1.9.1



[PATCH] fsl/qe: Do not prefix header guard with CONFIG_

2016-06-08 Thread Andreas Ziegler
The CONFIG_ prefix should only be used for options which
can be configured through Kconfig and not for guarding headers.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 include/soc/fsl/qe/qe_tdm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/soc/fsl/qe/qe_tdm.h b/include/soc/fsl/qe/qe_tdm.h
index 4c91498..a1664b6 100644
--- a/include/soc/fsl/qe/qe_tdm.h
+++ b/include/soc/fsl/qe/qe_tdm.h
@@ -11,8 +11,8 @@
  * option) any later version
  */
 
-#ifndef CONFIG_QE_TDM_H
-#define CONFIG_QE_TDM_H
+#ifndef _QE_TDM_H_
+#define _QE_TDM_H_
 
 #include 
 #include 
-- 
1.9.1



[PATCH] fsl/qe: Do not prefix header guard with CONFIG_

2016-06-08 Thread Andreas Ziegler
The CONFIG_ prefix should only be used for options which
can be configured through Kconfig and not for guarding headers.

Signed-off-by: Andreas Ziegler 
---
 include/soc/fsl/qe/qe_tdm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/soc/fsl/qe/qe_tdm.h b/include/soc/fsl/qe/qe_tdm.h
index 4c91498..a1664b6 100644
--- a/include/soc/fsl/qe/qe_tdm.h
+++ b/include/soc/fsl/qe/qe_tdm.h
@@ -11,8 +11,8 @@
  * option) any later version
  */
 
-#ifndef CONFIG_QE_TDM_H
-#define CONFIG_QE_TDM_H
+#ifndef _QE_TDM_H_
+#define _QE_TDM_H_
 
 #include 
 #include 
-- 
1.9.1



[PATCH] checkkconfigsymbols.py: Fix typo in help message

2016-03-31 Thread Andreas Ziegler
Fix a typo in the help message for the -d parameter by removing one 'm'.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 scripts/checkkconfigsymbols.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
index d8f6c09..df643f6 100755
--- a/scripts/checkkconfigsymbols.py
+++ b/scripts/checkkconfigsymbols.py
@@ -89,7 +89,7 @@ def parse_options():
 
 if opts.diff and not re.match(r"^[\w\-\.]+\.\.[\w\-\.]+$", opts.diff):
 sys.exit("Please specify valid input in the following format: "
- "\'commmit1..commit2\'")
+ "\'commit1..commit2\'")
 
 if opts.commit or opts.diff:
 if not opts.force and tree_is_dirty():
-- 
1.9.1



[PATCH] checkkconfigsymbols.py: Fix typo in help message

2016-03-31 Thread Andreas Ziegler
Fix a typo in the help message for the -d parameter by removing one 'm'.

Signed-off-by: Andreas Ziegler 
---
 scripts/checkkconfigsymbols.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/checkkconfigsymbols.py b/scripts/checkkconfigsymbols.py
index d8f6c09..df643f6 100755
--- a/scripts/checkkconfigsymbols.py
+++ b/scripts/checkkconfigsymbols.py
@@ -89,7 +89,7 @@ def parse_options():
 
 if opts.diff and not re.match(r"^[\w\-\.]+\.\.[\w\-\.]+$", opts.diff):
 sys.exit("Please specify valid input in the following format: "
- "\'commmit1..commit2\'")
+ "\'commit1..commit2\'")
 
 if opts.commit or opts.diff:
 if not opts.force and tree_is_dirty():
-- 
1.9.1



[PATCH] PCI: cleanup pci/pcie/Kconfig

2016-03-15 Thread Andreas Ziegler
This change cleans up style issues in drivers/pci/pcie/Kconfig, in
particular all indentation is now done using tabs, not spaces, and
the definition of PCIEASPM_DEBUG is now separated from the definition
of PCIEASPM with a newline.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 drivers/pci/pcie/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index e294713..72db7f4 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -44,6 +44,7 @@ config PCIEASPM
  /sys/module/pcie_aspm/parameters/policy
 
  When in doubt, say Y.
+
 config PCIEASPM_DEBUG
bool "Debug PCI Express ASPM"
depends on PCIEASPM
@@ -58,20 +59,20 @@ choice
depends on PCIEASPM
 
 config PCIEASPM_DEFAULT
-bool "BIOS default"
+   bool "BIOS default"
depends on PCIEASPM
help
  Use the BIOS defaults for PCI Express ASPM.
 
 config PCIEASPM_POWERSAVE
-bool "Powersave"
+   bool "Powersave"
depends on PCIEASPM
help
  Enable PCI Express ASPM L0s and L1 where possible, even if the
  BIOS did not.
 
 config PCIEASPM_PERFORMANCE
-bool "Performance"
+   bool "Performance"
depends on PCIEASPM
help
  Disable PCI Express ASPM L0s and L1, even if the BIOS enabled them.
-- 
1.9.1



[PATCH] PCI: cleanup pci/pcie/Kconfig

2016-03-15 Thread Andreas Ziegler
This change cleans up style issues in drivers/pci/pcie/Kconfig, in
particular all indentation is now done using tabs, not spaces, and
the definition of PCIEASPM_DEBUG is now separated from the definition
of PCIEASPM with a newline.

Signed-off-by: Andreas Ziegler 
---
 drivers/pci/pcie/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index e294713..72db7f4 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -44,6 +44,7 @@ config PCIEASPM
  /sys/module/pcie_aspm/parameters/policy
 
  When in doubt, say Y.
+
 config PCIEASPM_DEBUG
bool "Debug PCI Express ASPM"
depends on PCIEASPM
@@ -58,20 +59,20 @@ choice
depends on PCIEASPM
 
 config PCIEASPM_DEFAULT
-bool "BIOS default"
+   bool "BIOS default"
depends on PCIEASPM
help
  Use the BIOS defaults for PCI Express ASPM.
 
 config PCIEASPM_POWERSAVE
-bool "Powersave"
+   bool "Powersave"
depends on PCIEASPM
help
  Enable PCI Express ASPM L0s and L1 where possible, even if the
  BIOS did not.
 
 config PCIEASPM_PERFORMANCE
-bool "Performance"
+   bool "Performance"
depends on PCIEASPM
help
  Disable PCI Express ASPM L0s and L1, even if the BIOS enabled them.
-- 
1.9.1



[PATCH] ARC: Don't source drivers/pci/pcie/Kconfig ourselves

2016-03-15 Thread Andreas Ziegler
Commit 5f8fc43217a0 ("PCI: Include pci/pcie/Kconfig directly from
pci/Kconfig") in linux-next changed drivers/pci/Kconfig to include
drivers/pci/pcie/Kconfig itself, so that architectures do not need
to source both files themselves. ARC just recently gained PCI support
through commit 6b3fb77998dd ("ARC: Add PCI support"), but this change
was based on the old behaviour of the Kconfig files. This makes
Kconfig now spit out the following warnings:

drivers/pci/pcie/Kconfig:61:warning: choice value used outside its choice group
drivers/pci/pcie/Kconfig:67:warning: choice value used outside its choice group
drivers/pci/pcie/Kconfig:74:warning: choice value used outside its choice group

This change updates the Kconfig file for ARC, dropping the now
unnecessary 'source' statement, which makes the warning disappear.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 arch/arc/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 69d05f5..282efec 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -592,7 +592,6 @@ config PCI_SYSCALL
def_bool PCI
 
 source "drivers/pci/Kconfig"
-source "drivers/pci/pcie/Kconfig"
 
 endmenu
 
-- 
1.9.1



[PATCH] ARC: Don't source drivers/pci/pcie/Kconfig ourselves

2016-03-15 Thread Andreas Ziegler
Commit 5f8fc43217a0 ("PCI: Include pci/pcie/Kconfig directly from
pci/Kconfig") in linux-next changed drivers/pci/Kconfig to include
drivers/pci/pcie/Kconfig itself, so that architectures do not need
to source both files themselves. ARC just recently gained PCI support
through commit 6b3fb77998dd ("ARC: Add PCI support"), but this change
was based on the old behaviour of the Kconfig files. This makes
Kconfig now spit out the following warnings:

drivers/pci/pcie/Kconfig:61:warning: choice value used outside its choice group
drivers/pci/pcie/Kconfig:67:warning: choice value used outside its choice group
drivers/pci/pcie/Kconfig:74:warning: choice value used outside its choice group

This change updates the Kconfig file for ARC, dropping the now
unnecessary 'source' statement, which makes the warning disappear.

Signed-off-by: Andreas Ziegler 
---
 arch/arc/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 69d05f5..282efec 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -592,7 +592,6 @@ config PCI_SYSCALL
def_bool PCI
 
 source "drivers/pci/Kconfig"
-source "drivers/pci/pcie/Kconfig"
 
 endmenu
 
-- 
1.9.1



[PATCH] security: integrity: Remove select to deleted option PUBLIC_KEY_ALGO_RSA

2016-03-04 Thread Andreas Ziegler
Commit d43de6c780a8 ("akcipher: Move the RSA DER encoding check to
the crypto layer") removed the Kconfig option PUBLIC_KEY_ALGO_RSA,
but forgot to remove a 'select' to this option in the definition of
INTEGRITY_ASYMMETRIC_KEYS.

Let's remove the select, as it's ineffective now.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 security/integrity/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 979be65..da95658 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -35,7 +35,6 @@ config INTEGRITY_ASYMMETRIC_KEYS
default n
 select ASYMMETRIC_KEY_TYPE
 select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-select PUBLIC_KEY_ALGO_RSA
 select CRYPTO_RSA
 select X509_CERTIFICATE_PARSER
help
-- 
1.9.1



[PATCH] security: integrity: Remove select to deleted option PUBLIC_KEY_ALGO_RSA

2016-03-04 Thread Andreas Ziegler
Commit d43de6c780a8 ("akcipher: Move the RSA DER encoding check to
the crypto layer") removed the Kconfig option PUBLIC_KEY_ALGO_RSA,
but forgot to remove a 'select' to this option in the definition of
INTEGRITY_ASYMMETRIC_KEYS.

Let's remove the select, as it's ineffective now.

Signed-off-by: Andreas Ziegler 
---
 security/integrity/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 979be65..da95658 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -35,7 +35,6 @@ config INTEGRITY_ASYMMETRIC_KEYS
default n
 select ASYMMETRIC_KEY_TYPE
 select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-select PUBLIC_KEY_ALGO_RSA
 select CRYPTO_RSA
 select X509_CERTIFICATE_PARSER
help
-- 
1.9.1



Left-over select to PUBLIC_KEY_ALGO_RSA

2016-03-04 Thread Andreas Ziegler
Hi David,

your patch "akcipher: Move the RSA DER encoding check to the crypto
layer" showed up in linux-next today as commit d43de6c780a8 (that is,
next-20160304).

It removes the CONFIG_PUBLIC_KEY_ALGO_RSA option, but does leave one
reference to it in place inside security/integrity/Kconfig, in the
definition of CONFIG_INTEGRITY_ASYMMETRIC_KEYS.

As the corresponding option is gone, the select statement can safely be
removed. Should I prepare a simple patch for that?

I detected this by using scripts/checkkconfigsymbols on today's and
yesterday's linux-next trees (i.e., "./scripts/checkkconfigsymbols.py -d
next-20160303..next-20160304").

Best regards,

Andreas


Left-over select to PUBLIC_KEY_ALGO_RSA

2016-03-04 Thread Andreas Ziegler
Hi David,

your patch "akcipher: Move the RSA DER encoding check to the crypto
layer" showed up in linux-next today as commit d43de6c780a8 (that is,
next-20160304).

It removes the CONFIG_PUBLIC_KEY_ALGO_RSA option, but does leave one
reference to it in place inside security/integrity/Kconfig, in the
definition of CONFIG_INTEGRITY_ASYMMETRIC_KEYS.

As the corresponding option is gone, the select statement can safely be
removed. Should I prepare a simple patch for that?

I detected this by using scripts/checkkconfigsymbols on today's and
yesterday's linux-next trees (i.e., "./scripts/checkkconfigsymbols.py -d
next-20160303..next-20160304").

Best regards,

Andreas


[PATCH] security: Remove select to deleted KEYS_DEBUG_PROC_KEYS option

2016-01-26 Thread Andreas Ziegler
Commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted'
keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option.

This option 'select's KEYS_DEBUG_PROC_KEYS which has been removed
earlier by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional
if CONFIG_KEYS=y"). This patch removes the inoperative select.

Signed-off-by: Andreas Ziegler 
---
 security/integrity/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 21d7568..7543398 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -45,7 +45,6 @@ config INTEGRITY_TRUSTED_KEYRING
bool "Require all keys on the integrity keyrings be signed"
depends on SYSTEM_TRUSTED_KEYRING
depends on INTEGRITY_ASYMMETRIC_KEYS
-   select KEYS_DEBUG_PROC_KEYS
default y
help
   This option requires that all keys added to the .ima and
-- 
1.9.1



'select' on deleted KEYS_DEBUG_PROC_KEYS option

2016-01-26 Thread Andreas Ziegler
Hi Dmitry,

your commit f4dc37785e9b ("integrity: define '.evm' as a builtin
'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig
option, which has a 'select' to KEYS_DEBUG_PROC_KEYS.

This option, however, has been removed over a year ago by commit
dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y")
and -- besides the 'select' mentioned above -- only occurs inside
(outdated) defconfigs.

Should I prepare a simple patch removing the select?

Best regards,

Andreas


Re: [PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments

2016-01-26 Thread Andreas Ziegler
On 01/26/2016 16:08, Rosen, Rami wrote:
> Hi, 
> 
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct 
> sock_cgroup_data *skcd)
>  #endif
>  }
> 
> In this occasion, seems that maybe something else is also missing:
> Shouldn't it be hereafter : +#else /* !CONFIG_SOCK_CGROUP_DATA */ 
> instead ? 
>
> -#else/* CONFIG_CGROUP_DATA */
> +#else/* CONFIG_SOCK_CGROUP_DATA */

It seems that there is no real consensus among the developers for that
particular case:

ziegler@box:~/linux$ git grep "#else \/\* \!CONFIG_" | wc -l
327
ziegler@box:~/linux$ git grep "#else \/\* CONFIG_" | wc -l
564

I don't mind changing it, I'm just not sure if that's what we want.

Regards,

Andreas


Use of deleted Kconfig option B43_PCMCIA

2016-01-26 Thread Andreas Ziegler
Hi Rafał,

your commit 399500da18f7 ("ssb: pick PCMCIA host code support from b43
driver") removed the Kconfig option B43_PCMCIA from Kconfig, but left
one reference to it in b43/main.c, inside the b43_print_driverinfo()
function.

Can this #ifdef be removed, should it be replaced with an #ifdef using
CONFIG_SSB_PCMCIAHOST instead or should we do something entirely different?

Best regards,

Andreas


[PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments

2016-01-26 Thread Andreas Ziegler
Commit bd1060a1d671 ("sock, cgroup: add sock->sk_cgroup") added new
code guarded by an #ifdef CONFIG_SOCK_CGROUP_DATA to
include/linux/cgroup.h.

In the comments for the corresponding #else and #endifs, however,
the option is misspelled as CONFIG_CGROUP_DATA. Fix those comments.

Signed-off-by: Andreas Ziegler 
---
 include/linux/cgroup.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 2162dca..48ce12f 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct 
sock_cgroup_data *skcd)
 #endif
 }
 
-#else  /* CONFIG_CGROUP_DATA */
+#else  /* CONFIG_SOCK_CGROUP_DATA */
 
 static inline void cgroup_sk_alloc(struct sock_cgroup_data *skcd) {}
 static inline void cgroup_sk_free(struct sock_cgroup_data *skcd) {}
 
-#endif /* CONFIG_CGROUP_DATA */
+#endif /* CONFIG_SOCK_CGROUP_DATA */
 
 #endif /* _LINUX_CGROUP_H */
-- 
1.9.1



[PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments

2016-01-26 Thread Andreas Ziegler
Commit bd1060a1d671 ("sock, cgroup: add sock->sk_cgroup") added new
code guarded by an #ifdef CONFIG_SOCK_CGROUP_DATA to
include/linux/cgroup.h.

In the comments for the corresponding #else and #endifs, however,
the option is misspelled as CONFIG_CGROUP_DATA. Fix those comments.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 include/linux/cgroup.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 2162dca..48ce12f 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct 
sock_cgroup_data *skcd)
 #endif
 }
 
-#else  /* CONFIG_CGROUP_DATA */
+#else  /* CONFIG_SOCK_CGROUP_DATA */
 
 static inline void cgroup_sk_alloc(struct sock_cgroup_data *skcd) {}
 static inline void cgroup_sk_free(struct sock_cgroup_data *skcd) {}
 
-#endif /* CONFIG_CGROUP_DATA */
+#endif /* CONFIG_SOCK_CGROUP_DATA */
 
 #endif /* _LINUX_CGROUP_H */
-- 
1.9.1



Use of deleted Kconfig option B43_PCMCIA

2016-01-26 Thread Andreas Ziegler
Hi Rafał,

your commit 399500da18f7 ("ssb: pick PCMCIA host code support from b43
driver") removed the Kconfig option B43_PCMCIA from Kconfig, but left
one reference to it in b43/main.c, inside the b43_print_driverinfo()
function.

Can this #ifdef be removed, should it be replaced with an #ifdef using
CONFIG_SSB_PCMCIAHOST instead or should we do something entirely different?

Best regards,

Andreas


Re: [PATCH] cgroup: Fix misspelling of CONFIG_SOCK_CGROUP_DATA in comments

2016-01-26 Thread Andreas Ziegler
On 01/26/2016 16:08, Rosen, Rami wrote:
> Hi, 
> 
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -604,11 +604,11 @@ static inline struct cgroup *sock_cgroup_ptr(struct 
> sock_cgroup_data *skcd)
>  #endif
>  }
> 
> In this occasion, seems that maybe something else is also missing:
> Shouldn't it be hereafter : +#else /* !CONFIG_SOCK_CGROUP_DATA */ 
> instead ? 
>
> -#else/* CONFIG_CGROUP_DATA */
> +#else/* CONFIG_SOCK_CGROUP_DATA */

It seems that there is no real consensus among the developers for that
particular case:

ziegler@box:~/linux$ git grep "#else \/\* \!CONFIG_" | wc -l
327
ziegler@box:~/linux$ git grep "#else \/\* CONFIG_" | wc -l
564

I don't mind changing it, I'm just not sure if that's what we want.

Regards,

Andreas


'select' on deleted KEYS_DEBUG_PROC_KEYS option

2016-01-26 Thread Andreas Ziegler
Hi Dmitry,

your commit f4dc37785e9b ("integrity: define '.evm' as a builtin
'trusted' keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig
option, which has a 'select' to KEYS_DEBUG_PROC_KEYS.

This option, however, has been removed over a year ago by commit
dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional if CONFIG_KEYS=y")
and -- besides the 'select' mentioned above -- only occurs inside
(outdated) defconfigs.

Should I prepare a simple patch removing the select?

Best regards,

Andreas


[PATCH] security: Remove select to deleted KEYS_DEBUG_PROC_KEYS option

2016-01-26 Thread Andreas Ziegler
Commit f4dc37785e9b ("integrity: define '.evm' as a builtin 'trusted'
keyring") introduces the INTEGRITY_TRUSTED_KEYRING Kconfig option.

This option 'select's KEYS_DEBUG_PROC_KEYS which has been removed
earlier by commit dabd39cc2fb1 ("KEYS: Make /proc/keys unconditional
if CONFIG_KEYS=y"). This patch removes the inoperative select.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 security/integrity/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 21d7568..7543398 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -45,7 +45,6 @@ config INTEGRITY_TRUSTED_KEYRING
bool "Require all keys on the integrity keyrings be signed"
depends on SYSTEM_TRUSTED_KEYRING
depends on INTEGRITY_ASYMMETRIC_KEYS
-   select KEYS_DEBUG_PROC_KEYS
default y
help
   This option requires that all keys added to the .ima and
-- 
1.9.1



[PATCH v2] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig

2016-01-25 Thread Andreas Ziegler
Commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access
through GGTT on Braswell") depended upon a working stop_machine() and
so forced the selection of STOP_MACHINE. However, commit 86fffe4a61dd
("kernel: remove stop_machine() Kconfig dependency") removed the option
STOP_MACHINE from init/Kconfig and ensured that stop_machine()
universally works. Due to the order in which the patches were applied,
removing the select from DRM_I915 got lost during merging.

Remove the now obsolete select statement.

Signed-off-by: Andreas Ziegler 
Reviewed-by: Chris Wilson 
---
Changes to v1:
- Tell the story right, suggested by Chris Wilson

 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index fcd77b2..051eab3 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -10,7 +10,6 @@ config DRM_I915
# the shmem_readpage() which depends upon tmpfs
select SHMEM
select TMPFS
-   select STOP_MACHINE
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
-- 
1.9.1



[PATCH] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig

2016-01-25 Thread Andreas Ziegler
Commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig
dependency") removed the option STOP_MACHINE from init/Kconfig, but
missed a select to this option from the DRM_I915 option.

Remove this now obsolete select statement.

Signed-off-by: Andreas Ziegler 
---
 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index fcd77b2..051eab3 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -10,7 +10,6 @@ config DRM_I915
# the shmem_readpage() which depends upon tmpfs
select SHMEM
select TMPFS
-   select STOP_MACHINE
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
-- 
1.9.1



Select on now-gone Kconfig option STOP_MACHINE

2016-01-25 Thread Andreas Ziegler
Hi Chris,

your commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with
access through GGTT on Braswell") added a select to STOP_MACHINE to the
DRM_I915 Kconfig option.

In 4.5-rc1, your commit 86fffe4a61dd ("kernel: remove stop_machine()
Kconfig dependency") then removed STOP_MACHINE entirely, but didn't
delete the select from drivers/gpu/drm/i915/Kconfig.

Should I prepare a simple patch which removes the select?

Best regards,

Andreas


[PATCH] mm: Fix two typos in comments for to_vmem_altmap()

2016-01-25 Thread Andreas Ziegler
Commit 4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment
vmemmap_populate()"), introduced the to_vmem_altmap() function.

The comments in this function contain two typos (one misspelling
of the Kconfig option CONFIG_SPARSEMEM_VMEMMAP, and one missing
letter 'n'), let's fix them up.

Signed-off-by: Andreas Ziegler 
---
 kernel/memremap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/memremap.c b/kernel/memremap.c
index e517a16..2e84aa8 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -377,7 +377,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long 
memmap_start)
/*
 * 'memmap_start' is the virtual address for the first "struct
 * page" in this range of the vmemmap array.  In the case of
-* CONFIG_SPARSE_VMEMMAP a page_to_pfn conversion is simple
+* CONFIG_SPARSEMEM_VMEMMAP a page_to_pfn conversion is simple
 * pointer arithmetic, so we can perform this to_vmem_altmap()
 * conversion without concern for the initialization state of
 * the struct page fields.
@@ -386,7 +386,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long 
memmap_start)
struct dev_pagemap *pgmap;
 
/*
-* Uncoditionally retrieve a dev_pagemap associated with the
+* Unconditionally retrieve a dev_pagemap associated with the
 * given physical address, this is only for use in the
 * arch_{add|remove}_memory() for setting up and tearing down
 * the memmap.
-- 
1.9.1



[PATCH] mm: Fix two typos in comments for to_vmem_altmap()

2016-01-25 Thread Andreas Ziegler
Commit 4b94ffdc4163 ("x86, mm: introduce vmem_altmap to augment
vmemmap_populate()"), introduced the to_vmem_altmap() function.

The comments in this function contain two typos (one misspelling
of the Kconfig option CONFIG_SPARSEMEM_VMEMMAP, and one missing
letter 'n'), let's fix them up.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 kernel/memremap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/memremap.c b/kernel/memremap.c
index e517a16..2e84aa8 100644
--- a/kernel/memremap.c
+++ b/kernel/memremap.c
@@ -377,7 +377,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long 
memmap_start)
/*
 * 'memmap_start' is the virtual address for the first "struct
 * page" in this range of the vmemmap array.  In the case of
-* CONFIG_SPARSE_VMEMMAP a page_to_pfn conversion is simple
+* CONFIG_SPARSEMEM_VMEMMAP a page_to_pfn conversion is simple
 * pointer arithmetic, so we can perform this to_vmem_altmap()
 * conversion without concern for the initialization state of
 * the struct page fields.
@@ -386,7 +386,7 @@ struct vmem_altmap *to_vmem_altmap(unsigned long 
memmap_start)
struct dev_pagemap *pgmap;
 
/*
-* Uncoditionally retrieve a dev_pagemap associated with the
+* Unconditionally retrieve a dev_pagemap associated with the
 * given physical address, this is only for use in the
 * arch_{add|remove}_memory() for setting up and tearing down
 * the memmap.
-- 
1.9.1



[PATCH] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig

2016-01-25 Thread Andreas Ziegler
Commit 86fffe4a61dd ("kernel: remove stop_machine() Kconfig
dependency") removed the option STOP_MACHINE from init/Kconfig, but
missed a select to this option from the DRM_I915 option.

Remove this now obsolete select statement.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
---
 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index fcd77b2..051eab3 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -10,7 +10,6 @@ config DRM_I915
# the shmem_readpage() which depends upon tmpfs
select SHMEM
select TMPFS
-   select STOP_MACHINE
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
-- 
1.9.1



Select on now-gone Kconfig option STOP_MACHINE

2016-01-25 Thread Andreas Ziegler
Hi Chris,

your commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with
access through GGTT on Braswell") added a select to STOP_MACHINE to the
DRM_I915 Kconfig option.

In 4.5-rc1, your commit 86fffe4a61dd ("kernel: remove stop_machine()
Kconfig dependency") then removed STOP_MACHINE entirely, but didn't
delete the select from drivers/gpu/drm/i915/Kconfig.

Should I prepare a simple patch which removes the select?

Best regards,

Andreas


[PATCH v2] drm/i915: Remove select to deleted STOP_MACHINE from Kconfig

2016-01-25 Thread Andreas Ziegler
Commit 5bab6f60cb4d ("drm/i915: Serialise updates to GGTT with access
through GGTT on Braswell") depended upon a working stop_machine() and
so forced the selection of STOP_MACHINE. However, commit 86fffe4a61dd
("kernel: remove stop_machine() Kconfig dependency") removed the option
STOP_MACHINE from init/Kconfig and ensured that stop_machine()
universally works. Due to the order in which the patches were applied,
removing the select from DRM_I915 got lost during merging.

Remove the now obsolete select statement.

Signed-off-by: Andreas Ziegler <andreas.zieg...@fau.de>
Reviewed-by: Chris Wilson <ch...@chris-wilson.co.uk>
---
Changes to v1:
- Tell the story right, suggested by Chris Wilson

 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index fcd77b2..051eab3 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -10,7 +10,6 @@ config DRM_I915
# the shmem_readpage() which depends upon tmpfs
select SHMEM
select TMPFS
-   select STOP_MACHINE
select DRM_KMS_HELPER
select DRM_PANEL
select DRM_MIPI_DSI
-- 
1.9.1



Re: MIPS: BMIPS: Enable GZIP ramdisk and timed printks

2015-11-12 Thread Andreas Ziegler
Hi Florian,

your patch "MIPS: BMIPS: Enable GZIP ramdisk and timed printks"
showed up as commit fb9e5642aa9e in linux-next today (that is,
next-20151112). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit adds two entries to arch/mips/configs/bmips_stb_defconfig,
but one of them has a typo (line 37):

CONFIG_PRINK_TIME=y

which should instead say (notice the missing 'T'):

CONFIG_PRINTK_TIME=y

Not sure how this got two Reviewed-by's, as this simple mistake could
have been easily spotted by running scripts/checkkconfigsymbols.py on
the patch.

Regards,

Andreas

[0] https://cados.cs.fau.de
Original patchwork: https://patchwork.linux-mips.org/patch/11307/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: MIPS: BMIPS: Enable GZIP ramdisk and timed printks

2015-11-12 Thread Andreas Ziegler
Hi Florian,

your patch "MIPS: BMIPS: Enable GZIP ramdisk and timed printks"
showed up as commit fb9e5642aa9e in linux-next today (that is,
next-20151112). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit adds two entries to arch/mips/configs/bmips_stb_defconfig,
but one of them has a typo (line 37):

CONFIG_PRINK_TIME=y

which should instead say (notice the missing 'T'):

CONFIG_PRINTK_TIME=y

Not sure how this got two Reviewed-by's, as this simple mistake could
have been easily spotted by running scripts/checkkconfigsymbols.py on
the patch.

Regards,

Andreas

[0] https://cados.cs.fau.de
Original patchwork: https://patchwork.linux-mips.org/patch/11307/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0

2015-10-20 Thread Andreas Ziegler
Hi Jarkko,

your patch "tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0"
showed up as commit 399235dc6e95 in linux-next today (that is,
next-20151020). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit creates the following structure of #ifdef blocks in
drivers/char/tpm/tpm_tis.c following line 1088:

 #ifdef CONFIG_ACPI
 ...
 #ifdef CONFIG_PNP
 ...
 #endif
 ...
 #endif

Looking at the definition of CONFIG_ACPI at drivers/acpi/Kconfig, line
5, we see that ACPI unconditionally selects PNP, meaning that CONFIG_PNP
is always enabled if CONFIG_ACPI has been enabled.
Thus, the inner #ifdef statement can never evaluate to 'false' if the
outer #ifdef evaluates to true (i.e., CONFIG_ACPI is enabled), and
hence, the #ifdef is unnecessary.

The same situation holds for the nested structure following line 1124,
where the #ifdef CONFIG_PNP at line 1129 is unnecessary.

Is this correct or did we miss something?

Regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0

2015-10-20 Thread Andreas Ziegler
Hi Jarkko,

your patch "tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0"
showed up as commit 399235dc6e95 in linux-next today (that is,
next-20151020). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit creates the following structure of #ifdef blocks in
drivers/char/tpm/tpm_tis.c following line 1088:

 #ifdef CONFIG_ACPI
 ...
 #ifdef CONFIG_PNP
 ...
 #endif
 ...
 #endif

Looking at the definition of CONFIG_ACPI at drivers/acpi/Kconfig, line
5, we see that ACPI unconditionally selects PNP, meaning that CONFIG_PNP
is always enabled if CONFIG_ACPI has been enabled.
Thus, the inner #ifdef statement can never evaluate to 'false' if the
outer #ifdef evaluates to true (i.e., CONFIG_ACPI is enabled), and
hence, the #ifdef is unnecessary.

The same situation holds for the nested structure following line 1124,
where the #ifdef CONFIG_PNP at line 1129 is unnecessary.

Is this correct or did we miss something?

Regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: spi/bcm63xx: fix standard accessors and compile guard

2015-10-13 Thread Andreas Ziegler
Hi Jonas,

your patch "spi/bcm63xx: fix standard accessors and compile guard"
showed up as commit 682b5280bf00 in linux-next today (that is,
next-20151013). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit fixes two #ifdef statements in drivers/spi/spi-bcm63xx.c
which involve CONFIG_CPU_BIG_ENDIAN.

>From the Makefile at drivers/spi/Makefile (line 20), we can see that the
file can only be built when CONFIG_SPI_BCM63XX is set. In the
corresponding Kconfig file (drivers/spi/Kconfig, line 137),
CONFIG_SPI_BCM63XX is defined to depend on CONFIG_BCM63XX. The latter is
defined in arch/mips/Kconfig (line 199), and selects
CONFIG_SYS_SUPPORTS_BIG_ENDIAN (but not CONFIG_SYS_SUPPORTS_LITTLE_ENDIAN).

Finally, CONFIG_CPU_BIG_ENDIAN (at arch/mips/Kconfig, line 1136) depends
on CONFIG_SYS_SUPPORTS_BIG_ENDIAN, which means that if the source file
is to be compiled, CONFIG_CPU_BIG_ENDIAN is the only possible selection
in the endianness choice. Hence, the #ifdefs are unnecessary and could
possibly be removed.

Is this correct, or am I missing something?

Best regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: spi/bcm63xx: fix standard accessors and compile guard

2015-10-13 Thread Andreas Ziegler
Hi Jonas,

your patch "spi/bcm63xx: fix standard accessors and compile guard"
showed up as commit 682b5280bf00 in linux-next today (that is,
next-20151013). I noticed it because we (a research group from
Erlangen[0]) are running daily checks on linux-next.

Your commit fixes two #ifdef statements in drivers/spi/spi-bcm63xx.c
which involve CONFIG_CPU_BIG_ENDIAN.

>From the Makefile at drivers/spi/Makefile (line 20), we can see that the
file can only be built when CONFIG_SPI_BCM63XX is set. In the
corresponding Kconfig file (drivers/spi/Kconfig, line 137),
CONFIG_SPI_BCM63XX is defined to depend on CONFIG_BCM63XX. The latter is
defined in arch/mips/Kconfig (line 199), and selects
CONFIG_SYS_SUPPORTS_BIG_ENDIAN (but not CONFIG_SYS_SUPPORTS_LITTLE_ENDIAN).

Finally, CONFIG_CPU_BIG_ENDIAN (at arch/mips/Kconfig, line 1136) depends
on CONFIG_SYS_SUPPORTS_BIG_ENDIAN, which means that if the source file
is to be compiled, CONFIG_CPU_BIG_ENDIAN is the only possible selection
in the endianness choice. Hence, the #ifdefs are unnecessary and could
possibly be removed.

Is this correct, or am I missing something?

Best regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Weird Kconfig condition (IPV6 [=n] || IPV6 [=n]=n)

2015-09-14 Thread Andreas Ziegler
Hi,

On 09/14/2015 10:41, Mason wrote:
> Hello,
> 
> I did make menuconfig in v4.2
> 
> In the help for CONFIG_CRYPTO_HMAC, I have
> 
>   │   Selected by: IP_SCTP [=n] && NET [=y] && INET [=y] && (IPV6 [=n] || 
> IPV6 [=n]=n) || INET_AH [=n] && NET [=y] && INET [=y] || \  │  
>   │ INET_ESP [=n] && NET [=y] && INET [=y] || INET6_AH [=n] && NET [=y] && 
> INET [=y] && IPV6 [=n] || INET6_ESP [=n] && NET [=y] && INET [=y]\ │  
>   │  && IPV6 [=n] || SCTP_COOKIE_HMAC_MD5 [=n] && NET [=y] && IP_SCTP [=n] && 
> SCTP_COOKIE_HMAC_MD5 [=n] || SCTP_COOKIE_HMAC_SHA1 [=n] && \│  
>   │ NET [=y] && IP_SCTP [=n] && SCTP_COOKIE_HMAC_SHA1 [=n] || CIFS [=n] && 
> NETWORK_FILESYSTEMS [=y] && INET [=y] || TRUSTED_KEYS [=n] && \│  
>   │ KEYS [=y] && TCG_TPM [=n] || ENCRYPTED_KEYS [=n] && KEYS [=y] || IMA [=n] 
> && INTEGRITY [=n] || EVM [=n] && INTEGRITY [=n] || \│  
>   │ CRYPTO_DRBG_HMAC [=y] && CRYPTO [=y] && CRYPTO_DRBG_MENU [=m] || 
> CRYPTO_DEV_OMAP_SHAM [=n] && CRYPTO [=y] && CRYPTO_HW [=y] && \  
> │  
>   │ ARCH_OMAP2PLUS [=n] || CRYPTO_DEV_QAT [=n] && CRYPTO [=y] && CRYPTO_HW 
> [=y]   │  
> 
> 
> What does (IPV6 [=n] || IPV6 [=n]=n) mean?
> Is this a bug in my configuration?

No, this comes from the definition of IP_SCTP in net/sctp/Kconfig.
 menuconfig IP_SCTP
  tristate "The SCTP Protocol"
  depends on INET
  depends on IPV6 || IPV6=n
 ^^

This weird-looking condition has the effect that:
- if IPV6 is disabled, IP_SCTP can be anything ("y","m" or "n")
- if IPV6 is enabled to "y", IP_SCTP can also be set to anything.
- (and this is the reason why it's there:) if IPV6 is a module ("m"),
IP_SCTP can only be "m" or "n".

In a way, the "depends on" does not only reference the symbol, but also
keeps track of the 'state' of the symbol it references, such that if the
dependency (here: IPV6) is to be built as a loadable module only, the
dependant feature can also only be built as a loadable module.

I can't find the Documentation for that at the moment, but I'm sure that
I read this at some point, maybe also only on a mailing list...

Regards,

Andreas

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: Weird Kconfig condition (IPV6 [=n] || IPV6 [=n]=n)

2015-09-14 Thread Andreas Ziegler
Hi,

On 09/14/2015 10:41, Mason wrote:
> Hello,
> 
> I did make menuconfig in v4.2
> 
> In the help for CONFIG_CRYPTO_HMAC, I have
> 
>   │   Selected by: IP_SCTP [=n] && NET [=y] && INET [=y] && (IPV6 [=n] || 
> IPV6 [=n]=n) || INET_AH [=n] && NET [=y] && INET [=y] || \  │  
>   │ INET_ESP [=n] && NET [=y] && INET [=y] || INET6_AH [=n] && NET [=y] && 
> INET [=y] && IPV6 [=n] || INET6_ESP [=n] && NET [=y] && INET [=y]\ │  
>   │  && IPV6 [=n] || SCTP_COOKIE_HMAC_MD5 [=n] && NET [=y] && IP_SCTP [=n] && 
> SCTP_COOKIE_HMAC_MD5 [=n] || SCTP_COOKIE_HMAC_SHA1 [=n] && \│  
>   │ NET [=y] && IP_SCTP [=n] && SCTP_COOKIE_HMAC_SHA1 [=n] || CIFS [=n] && 
> NETWORK_FILESYSTEMS [=y] && INET [=y] || TRUSTED_KEYS [=n] && \│  
>   │ KEYS [=y] && TCG_TPM [=n] || ENCRYPTED_KEYS [=n] && KEYS [=y] || IMA [=n] 
> && INTEGRITY [=n] || EVM [=n] && INTEGRITY [=n] || \│  
>   │ CRYPTO_DRBG_HMAC [=y] && CRYPTO [=y] && CRYPTO_DRBG_MENU [=m] || 
> CRYPTO_DEV_OMAP_SHAM [=n] && CRYPTO [=y] && CRYPTO_HW [=y] && \  
> │  
>   │ ARCH_OMAP2PLUS [=n] || CRYPTO_DEV_QAT [=n] && CRYPTO [=y] && CRYPTO_HW 
> [=y]   │  
> 
> 
> What does (IPV6 [=n] || IPV6 [=n]=n) mean?
> Is this a bug in my configuration?

No, this comes from the definition of IP_SCTP in net/sctp/Kconfig.
 menuconfig IP_SCTP
  tristate "The SCTP Protocol"
  depends on INET
  depends on IPV6 || IPV6=n
 ^^

This weird-looking condition has the effect that:
- if IPV6 is disabled, IP_SCTP can be anything ("y","m" or "n")
- if IPV6 is enabled to "y", IP_SCTP can also be set to anything.
- (and this is the reason why it's there:) if IPV6 is a module ("m"),
IP_SCTP can only be "m" or "n".

In a way, the "depends on" does not only reference the symbol, but also
keeps track of the 'state' of the symbol it references, such that if the
dependency (here: IPV6) is to be built as a loadable module only, the
dependant feature can also only be built as a loadable module.

I can't find the Documentation for that at the moment, but I'm sure that
I read this at some point, maybe also only on a mailing list...

Regards,

Andreas

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs

2015-09-03 Thread Andreas Ziegler
Hi Helge,

today's linux-next tree (next-20150903) contains commit 20f924902ff6
("parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs")
which you authored.

I noticed it because we[0] are running a daily analysis on all commits
in linux-next as part of our research and our tools reported it.

In the patch, you create the following #if defined() structure in
arch/parisc/include/asm/cache.h (lines 16 and following):

 #if defined(CONFIG_PA8X00)
  ...
 #elif defined(CONFIG_PA20)
  ...
 #else
  ...
 #endif

In Kconfig, CONFIG_PA20 is defined as the following
(arch/parisc/Kconfig, line 163):

config PA20
  def_bool y
  depends on PA8X00

This means that CONFIG_PA20 can and will only be enabled if
CONFIG_PA8X00 has already been enabled, which means that the contents of
the "#elif defined(CONFIG_PA20)" block can never be reached: its
condition is only evaluated if CONFIG_PA8X00 is disabled, but then
CONFIG_PA20 can never be enabled either.

Best regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs

2015-09-03 Thread Andreas Ziegler
Hi Helge,

today's linux-next tree (next-20150903) contains commit 20f924902ff6
("parisc: adjust L1_CACHE_BYTES to 128 bytes on PA8800 and PA8900 CPUs")
which you authored.

I noticed it because we[0] are running a daily analysis on all commits
in linux-next as part of our research and our tools reported it.

In the patch, you create the following #if defined() structure in
arch/parisc/include/asm/cache.h (lines 16 and following):

 #if defined(CONFIG_PA8X00)
  ...
 #elif defined(CONFIG_PA20)
  ...
 #else
  ...
 #endif

In Kconfig, CONFIG_PA20 is defined as the following
(arch/parisc/Kconfig, line 163):

config PA20
  def_bool y
  depends on PA8X00

This means that CONFIG_PA20 can and will only be enabled if
CONFIG_PA8X00 has already been enabled, which means that the contents of
the "#elif defined(CONFIG_PA20)" block can never be reached: its
condition is only evaluated if CONFIG_PA8X00 is disabled, but then
CONFIG_PA20 can never be enabled either.

Best regards,

Andreas

[0] https://cados.cs.fau.de
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/