arch: s390: appldata: using strncpy() instead of sprintf()
Subsystems (4)
PATCH 1/1 arch: s390: appldata: using strncpy() instead of sprintf()
'buf[2]' is 2 bytes length, and sprintf() will append '\0' at the end of string "?\n", so original implementation is memory overflow.
Need use strncpy() instead of sprintf().
--- arch/s390/appldata/appldata_base.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
arch/s390/appldata/appldata_base.c +3 -2
@@ -212,7 +212,8 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); + len =strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +sizeof (buf));if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len))
@@ -317,7 +318,7 @@ appldata_generic_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, ops->active ?"1\n" :"0\n" ); + len =strncpy (buf, ops->active ?"1\n" :"0\n" ,sizeof (buf));if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len)) {
Signature · 1 line
1.7.7.6
Quoted 2 lines · Chen Gang
'buf[2]' is 2 bytes length, and
sprintf()will append '\0' at the end of string "?\n", so original implementation is memory overflow.
Nice catch!
Quoted 1 line
Need use
strncpy()instead ofsprintf().
Or memcpy(), as the string is always 2 bytes?
Of course that would break future extension with multi-digit responses, but in that case, the buffer size needs to be expanded, too.
Anyway, this code needed some more brain cycles to actually understand what's it doing, and whether it's correct. So I guess it's ripe for more refactoring...
Quoted 10 lines
Signed-off-by Chen Gang <[email protected]>--- arch/s390/appldata/appldata_base.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)arch/s390/appldata/appldata_base.c +0
-0
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 215
Quoted 4 lines
arch/s390/appldata/appldata_base.c +0
-1@@ -212,7 +212,8 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" );
Before, len was always 2. The string was zero-terminated but the zero was not returned to user space, which is OK.
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 216
Quoted 2 lines
arch/s390/appldata/appldata_base.c +2
-0+ len =strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +sizeof (buf));
Oops, strncpy() doesn't return a size, but a pointer! So this doesn't work. I guess you haven't compiled this?
http://kernel.org/pub/tools/crosstool/files/bin/
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 219
Quoted 3 lines
arch/s390/appldata/appldata_base.c +0
-0if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len))
What about creating a "uprintf()" that formats strings to a userspace buffer?
arch/s390/appldata/appldata_base.c appldata_generic_handler(ctl_table *ctl, int write, · line 321
Quoted 6 lines
arch/s390/appldata/appldata_base.c +1
-1@@ -317,7 +318,7 @@ appldata_generic_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, ops->active ?"1\n" :"0\n" ); + len =strncpy (buf, ops->active ?"1\n" :"0\n" ,sizeof (buf));
Same here.
arch/s390/appldata/appldata_base.c appldata_generic_handler(ctl_table *ctl, int write, · line 324
Quoted 3 lines
arch/s390/appldata/appldata_base.c +0
-0if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len)) {
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Quoted 5 lines · Geert Uytterhoeven
Quoted 5 lines
Signed-off-by Chen Gang <[email protected]>Quoted 4 lines
--- arch/s390/appldata/appldata_base.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
Quoted 12 lines
Quoted 9 lines
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 215
Quoted 9 lines
arch/s390/appldata/appldata_base.c +0
-1@@ -212,7 +212,8 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" );Before, len was always 2. The string was zero-terminated but the zero was not returned to user space, which is OK.
I guess, the original API did not treat it as a zero-terminated string, it is more like a 'protocol' contents for 'write' or '!write' between kernel mode and user mode.
At least the kernel did not treat it as a zero-terminated string, we can reference the '!write' work flow to know about it.
Quoted 6 lines
Quoted 2 lines
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 216
Quoted 2 lines
arch/s390/appldata/appldata_base.c +2
-0+ len =strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +sizeof (buf));Oops,
strncpy()doesn't return a size, but a pointer! So this doesn't work. I guess you haven't compiled this?
http://kernel.org/pub/tools/crosstool/files/bin/
Oh, it is my fault, I need compile it for a basic checking.
Quoted 5 lines
Quoted 3 lines
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 219
Quoted 3 lines
arch/s390/appldata/appldata_base.c +0
-0if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len))What about creating a "
uprintf()" that formats strings to a userspace buffer?
Hmm... maybe it is a good idea for processing the zero-terminated string from kernel mode to user mode, but I guess, for our case, it is not a zero-terminated string, so...
All together, thank you very much for your checking, I should send patch v2.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
PATCH 1/1 arch: s390: include: asm: typo issue for the redundency comma, found by cross compiling.
It is a typo issue, remove the redundancy comma.
--- arch/s390/include/asm/pgtable.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
arch/s390/include/asm/pgtable.h +1 -1
@@ -1351,7 +1351,7 @@ static inline pmd_t pmd_mkwrite(pmd_t pmd)#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define __HAVE_ARCH_PGTABLE_DEPOSIT -extern void pgtable_trans_huge_deposit(struct mm_struct *mm, , pmd_t *pmdp, +extern void pgtable_trans_huge_deposit (struct mm_struct *mm,pmd_t *pmdp,pgtable_t pgtable);#define __HAVE_ARCH_PGTABLE_WITHDRAW
Signature · 1 line
1.7.7.6
PATCH v2 1/1 arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
In reply to Geert Uytterhoeven'buf[2]' is 2 bytes length, and sprintf() will append '\0' at the end of string "?\n", so original implementation is memory overflow.
Need use strncpy() and strnlen() instead of sprintf().
--- arch/s390/appldata/appldata_base.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
arch/s390/appldata/appldata_base.c +5 -2
@@ -212,7 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); +strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +ARRAY_SIZE (buf)); + len =strnlen (buf,ARRAY_SIZE (buf));if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len))
@@ -317,7 +319,8 @@ appldata_generic_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, ops->active ?"1\n" :"0\n" ); +strncpy (buf, ops->active ?"1\n" :"0\n" ,ARRAY_SIZE (buf)); + len =strnlen (buf,ARRAY_SIZE (buf));if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len)) {
Signature · 1 line
1.7.7.6
Gerald Schaefer
AppliedRe: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 8 lines · On Mon, 27 May 2013 17:55:13 +0800 Chen Gang <[email protected]> wrote:
'buf[2]' is 2 bytes length, and
sprintf()will append '\0' at the end of string "?\n", so original implementation is memory overflow.Need use
strncpy()andstrnlen()instead ofsprintf().Signed-off-by Chen Gang <[email protected]>
Applied, thanks.
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 4 lines · Gerald Schaefer
Quoted 1 line · On Mon, 27 May 2013 17:55:13 +0800 Chen Gang <[email protected]> wrote:
Quoted 1 line
Quoted 9 lines
Quoted 7 lines
Quoted 7 lines
'buf[2]' is 2 bytes length, and
sprintf()will append '\0' at the end of string "?\n", so original implementation is memory overflow.Need use
strncpy()andstrnlen()instead ofsprintf().Signed-off-by Chen Gang <[email protected]>Applied, thanks.
Thank you, too.
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
In reply to Chen GangAm 27.05.2013 11:55, schrieb Chen Gang:
Quoted 17 lines
'buf[2]' is 2 bytes length, and
sprintf()will append '\0' at the end of string "?\n", so original implementation is memory overflow.Need use
strncpy()andstrnlen()instead ofsprintf().Signed-off-by Chen Gang <[email protected]>--- arch/s390/appldata/appldata_base.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)arch/s390/appldata/appldata_base.c +0
-0
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 216
Quoted 6 lines
arch/s390/appldata/appldata_base.c +2
-1@@ -212,7 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); +strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +ARRAY_SIZE (buf));
an other way would be
buf[]="0\n";
if (appldata_timer_active) buf[0]='1';
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 1 line
arch/s390/appldata/appldata_base.c +1
-0+ len =strnlen (buf,ARRAY_SIZE (buf));
can len ever change ?
re,
wh
Quoted 13 lines
arch/s390/appldata/appldata_base.c +2
-1if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len))@@ -317,7 +319,8 @@ appldata_generic_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, ops->active ?"1\n" :"0\n" ); +strncpy (buf, ops->active ?"1\n" :"0\n" ,ARRAY_SIZE (buf)); + len =strnlen (buf,ARRAY_SIZE (buf));if (len > *lenp) len = *lenp;if (copy_to_user (buffer, buf, len)) {
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
In reply to Chen GangQuoted 15 lines · walter harms
Am 27.05.2013 11:55, schrieb Chen Gang:
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 216
Quoted 11 lines
arch/s390/appldata/appldata_base.c +2
-1@@ -212,7 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); +strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +ARRAY_SIZE (buf));an other way would be
Quoted 8 lines
buf[]="0\n";
if (appldata_timer_active) buf[0]='1';arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 1 line
arch/s390/appldata/appldata_base.c +1
-0+ len =strnlen (buf,ARRAY_SIZE (buf));can len ever change ?
I do not know whether it will be changed, but we need treat it that it may be changed in the future.
I guess it uses "?\n" as a protocol between kernel mode and user mode. This patch fix the 'protocol' transferring issue independent of the 'protocol' itself.
In the future, the 'protocol' updates to new version (e.g. append additional characters, which will still compitable with the original version), what this patch have done will be still effective.
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
In reply to Chen Gangarch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 4 lines · Chen Gang
arch/s390/appldata/appldata_base.c +3
-1- len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); +strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +ARRAY_SIZE (buf)); + len =strnlen (buf,ARRAY_SIZE (buf));
Since the strings here are a fixed, preknown size, you should use memcpy (or just fill in the array directly which would likely take fewer instructions since the strings are so short and vary by one character) and hard-code the length or use sizeof() instead of strnlen().
David
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 10 lines · David Howells
Quoted 4 lines · Chen Gang
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 4 lines
arch/s390/appldata/appldata_base.c +3
-1- len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); +strncpy (buf, appldata_timer_active ?"1\n" :"0\n" , +ARRAY_SIZE (buf)); + len =strnlen (buf,ARRAY_SIZE (buf));Since the strings here are a fixed, preknown size, you should use memcpy (or just fill in the array directly which would likely take fewer instructions since the strings are so short and vary by one character) and hard-code the length or use
sizeof()instead ofstrnlen().
I treat it "?\n" as a 'protocol' between kernel mode and user mode, the original issue is about transferring 'protocol' data. the patch wants to fix the issue, independent of 'protocol'.
appldata_timer_handler() itself has already separated "transferring 'protocol' data" from "processing 'protocol' data" (but not let the "processing 'protocol' data" in another new function)
Your suggestion will improve the speed, but may merge "transferring 'protocol' data" and "processing 'protocol' data" together.
So if the performance is not quite important in this location, it is still reasonable to follow with the original author's willing (especially what he/she has done sounds reasonable too).
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 2 lines · Chen Gang
Your suggestion will improve the speed, but may merge "transferring 'protocol' data" and "processing 'protocol' data" together.
Look at it this way: You're having to step very carefully because you are fully expecting the strings not to be NUL-terminated. Therefore you probably avoid using string functions if you can.
In fact, looking at the code, why are you copying the data through an intermediate buffer at all? Why not just copy directly to userspace:
int len;
char buf[2];
if (!*lenp || *ppos) {
*lenp = 0;
return 0;
}
if (!write) {
- len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n");
+ const char *ptr = appldata_timer_active ? "1\n" : "0\n";
+ size_t len = 2;
if (len > *lenp)
len = *lenp;
if (copy_to_user(buffer, buf, len))
return -EFAULT;
goto out;
}Put like that, it's fairly obvious what is going on.
David
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 30 lines · On Tue, 28 May 2013 13:17:09 +0100 David Howells <[email protected]> wrote:
Quoted 2 lines · Chen Gang
Your suggestion will improve the speed, but may merge "transferring 'protocol' data" and "processing 'protocol' data" together.
Look at it this way: You're having to step very carefully because you are fully expecting the strings not to be NUL-terminated. Therefore you probably avoid using string functions if you can.
In fact, looking at the code, why are you copying the data through an intermediate buffer at all? Why not just copy directly to userspace:
int len; char buf[2]; if (!*lenp || *ppos) { *lenp = 0; return 0; } if (!write) {(quoted diff) +2
-1- len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n"); + const char *ptr = appldata_timer_active ? "1\n" : "0\n"; + size_t len = 2;if (len > *lenp) len = *lenp; if (copy_to_user(buffer, buf, len)) return -EFAULT; goto out; }
Quoted 1 line
Put like that, it's fairly obvious what is going on.
Yes, we could do that for !write, but we can't get rid of the buffer completely, as we need it for the other case and copy_from_user(). I have already applied the v2 version from Chen Gang, as it fixes the overflow bug, and the affected code is not performance critical at all. I like the improved readability of your approach, but the patch is already on its way to Martins "for-linus" branch, and I think it's "good enough".
Thanks for all the feedback to this little piece of very old (and partly ugly) code from my very first Linux device driver :-)
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 41 lines · Gerald Schaefer
Quoted 30 lines · On Tue, 28 May 2013 13:17:09 +0100 David Howells <[email protected]> wrote:
Quoted 30 lines
Quoted 2 lines · Chen Gang
Quoted 2 lines
Your suggestion will improve the speed, but may merge "transferring 'protocol' data" and "processing 'protocol' data" together.
Look at it this way: You're having to step very carefully because you are fully expecting the strings not to be NUL-terminated. Therefore you probably avoid using string functions if you can.
In fact, looking at the code, why are you copying the data through an intermediate buffer at all? Why not just copy directly to userspace:
int len; char buf[2]; if (!*lenp || *ppos) { *lenp = 0; return 0; } if (!write) {(quoted diff) +2
-1- len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n"); + const char *ptr = appldata_timer_active ? "1\n" : "0\n"; + size_t len = 2;if (len > *lenp) len = *lenp; if (copy_to_user(buffer, buf, len)) return -EFAULT; goto out; }Quoted 1 line
Quoted 1 line
Put like that, it's fairly obvious what is going on.
Yes, we could do that for !write, but we can't get rid of the buffer completely, as we need it for the other case and
copy_from_user(). I have already applied the v2 version from Chen Gang, as it fixes the overflow bug, and the affected code is not performance critical at all. I like the improved readability of your approach, but the patch is already on its way to Martins "for-linus" branch, and I think it's "good enough".
Quoted 3 lines
Thanks for all the feedback to this little piece of very old (and partly ugly) code from my very first Linux device driver :-)
OK, thank you. and thank all related feedback from Geert, Walter, David
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
In reply to David HowellsQuoted 23 lines · David Howells
Quoted 2 lines · Chen Gang
Your suggestion will improve the speed, but may merge "transferring 'protocol' data" and "processing 'protocol' data" together.
Look at it this way: You're having to step very carefully because you are fully expecting the strings not to be NUL-terminated. Therefore you probably avoid using string functions if you can.
In fact, looking at the code, why are you copying the data through an intermediate buffer at all? Why not just copy directly to userspace:
int len; char buf[2]; if (!*lenp || *ppos) { *lenp = 0; return 0; } if (!write) {(quoted diff) +2
-1- len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n"); + const char *ptr = appldata_timer_active ? "1\n" : "0\n"; + size_t len = 2;
Quoted 3 lines
if (len > *lenp)
len = *lenp;
if (copy_to_user(buffer, buf, len))
may: "if (copy_to_user(buffer, ptr, len))" ?
:-)
Quoted 6 lines
return -EFAULT; goto out; }Put like that, it's fairly obvious what is going on.
It seems well fixed (still independent of processing 'protocol' data).
:-)
And how about below:
-------------------------------diff begin------------------------------
arch/s390/appldata/appldata_base.c +3 -4
@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); -if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , + min(2, *lenp))return -EFAULT ; goto out; }
-------------------------------diff end--------------------------------
Thanks
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 5 lines · Chen Gang
arch/s390/appldata/appldata_base.c +0
-0
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 10 lines
arch/s390/appldata/appldata_base.c +3
-4@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); -if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , + min(2, *lenp))
I don't have a cross compiler set up, but this will generate a warning, I think. min_t() is needed.
regards,
dan carpenter
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 11 lines · Dan Carpenter
Quoted 9 lines · Chen Gang
Quoted 9 lines
arch/s390/appldata/appldata_base.c +0
-1@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" );
Quoted 8 lines
Quoted 6 lines
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write, · line 217
Quoted 6 lines
arch/s390/appldata/appldata_base.c +3
-3-if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , + min(2, *lenp))I don't have a cross compiler set up, but this will generate a warning, I think.
min_t()is needed.
Yes, it even can not pass compiling: need additional ')' for 'if'.
The new fix is below, (for pass compiling, also need fix the incorrect declaration, which I have already sent an individual patch for it).
-----------------------------diff v2 begin------------------------------
(quoted diff) +0 -0
b/arch/s390/appldata/appldata_base.c
index bae0f40..b0915b4 100644
arch/s390/appldata/appldata_base.c +3 -4
@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); -if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , +min_t (size_t,2 , *lenp)))return -EFAULT ; goto out; }
(quoted diff) +0 -0
b/arch/s390/include/asm/pgtable.h
index 172209c..ed88890 100644
arch/s390/include/asm/pgtable.h +1 -1
@@ -1348,7 +1348,7 @@ static inline pmd_t pmd_mkwrite(pmd_t pmd)#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define __HAVE_ARCH_PGTABLE_DEPOSIT -extern void pgtable_trans_huge_deposit(struct mm_struct *mm, , pmd_t *pmdp, +extern void pgtable_trans_huge_deposit (struct mm_struct *mm,pmd_t *pmdp,pgtable_t pgtable);#define __HAVE_ARCH_PGTABLE_WITHDRAW
-----------------------------diff v2 end--------------------------------
For cross compiler, I use fedora 16/17, can yum update binutils-* to get many platform cross-compilers (s390, powerpc, arm, xtensa, ...).
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation
Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()
Quoted 21 lines · Chen Gang
Quoted 11 lines · Dan Carpenter
Quoted 9 lines · Chen Gang
Quoted 9 lines
arch/s390/appldata/appldata_base.c +0
-1@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" );Quoted 8 lines
Quoted 6 lines
arch/s390/appldata/appldata_base.c appldata_timer_handler(ctl_table *ctl, int write,
Quoted 6 lines
arch/s390/appldata/appldata_base.c +3
-3-if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , + min(2, *lenp))I don't have a cross compiler set up, but this will generate a warning, I think.
min_t()is needed.
Quoted 5 lines
Yes, it even can not pass compiling: need additional ')' for 'if'.
The new fix is below, (for pass compiling, also need fix the incorrect declaration, which I have already sent an individual patch for it).
Excuse me, the diff v2 is word wrap, if really need it, please edit it.
If necessary, I will send the related patch for it.
Thanks.
Quoted 46 lines
-----------------------------diff v2 begin------------------------------
arch/s390/appldata/appldata_base.c +3
-4@@ -212,10 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,return 0 ; }if (!write) { - len =sprintf (buf, appldata_timer_active ?"1\n" :"0\n" ); -if (len > *lenp) - len = *lenp; -if (copy_to_user (buffer, buf, len)) +if (copy_to_user (buffer, + appldata_timer_active ?"1\n" :"0\n" , +min_t (size_t,2 , *lenp)))return -EFAULT ; goto out; }arch/s390/include/asm/pgtable.h +1
-2@@ -1348,7 +1348,7 @@ static inline pmd_t pmd_mkwrite(pmd_t pmd)#ifdef CONFIG_TRANSPARENT_HUGEPAGE #define __HAVE_ARCH_PGTABLE_DEPOSIT -extern void pgtable_trans_huge_deposit(struct mm_struct *mm, , pmd_t *pmdp, +extern void pgtable_trans_huge_deposit (struct mm_struct *mm,pmd_t *pmdp,pgtable_t pgtable);#define __HAVE_ARCH_PGTABLE_WITHDRAW -----------------------------diff v2 end--------------------------------For cross compiler, I use fedora 16/17, can yum update binutils-* to get many platform cross-compilers (s390, powerpc, arm, xtensa, ...).
Thanks.
Signature · 3 lines
Chen Gang Asianux Corporation