Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On September 15, 2015 4:30:56 AM CDT, Sean Fu wrote: >On Mon, Sep 14, 2015 at 12:44 AM, Eric W. Biederman > wrote: >> Sean Fu writes: >> >>> On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman >>> wrote: Sean Fu writes: >> '\0' is not and has never been valid in a text file. >> proc files are a text interface. >> Expecting '\0' to be accepted is very strange, and apparently there >is >> only one program in existence that does. >> >> That a trailing '\0' was ever accepted was due to a bug in the code. >> >> Accepting '\0' in general in a text interface is a very dangerous and >> buggy pattern so it must be done very carefully or else other >> regressions or bugs could be easily introduced. >Ok, >Could you please give me more detail about the potential risk from the >patch? Regressions. AKA There may now be programs that depend on writing a '\0' failing. >What is the different behavior between the patch and old kernel? >It seems like entirely same. Not at all. And clear explanations have already been given. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 15 Sep 2015 17:10:11 +0800 Sean Fu wrote: > According to POSIX standard, "The write() function shall attempt to > write nbyte bytes from the buffer pointed to by buf to the file > associated with the open file descriptor, fildes.". > So it is not the length of string(strlen). Why do we care about the "write()" function? Yeah, so be it. Send out as much data as you want. We care about the effects of the proc file system by the input it receives. You can send in X amounts of data. That's not broken. The only apps that would send out a string to the proc filesystem that includes "\0" and beyond is root kits. I'm sorry but your arguments are starting to go south. I'm starting to think that the app that broke is a root kit, and maybe it's best to just Nack this. -- Steve > > On Mon, Sep 14, 2015 at 4:05 AM, Steven Rostedt wrote: > > On Sun, 13 Sep 2015 20:39:31 +0800 > > Sean Fu wrote: > > > > > >> > Accepting a '\0' is not at all reasonable for a text interface. The > >> > application that does it is buggy. > >> It is hard to comprehend that the current kernel can accept two bytes > >> "1 ", "1\t", "1\n" except "1\0". > > > > Um, it does not seem hard to comprehend at all. As this is a string, > > and it acts the same as a printf() or strlen. > > > > strlen("1 ") == 2 > > strlen("1\t") == 2 > > strlen("1\n") == 2 > > strlen("1\0") == 1 > > > > Big difference to me. > > > > -- Steve > > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Mon, Sep 14, 2015 at 12:44 AM, Eric W. Biederman wrote: > Sean Fu writes: > >> On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman >> wrote: >>> Sean Fu writes: > '\0' is not and has never been valid in a text file. > proc files are a text interface. > Expecting '\0' to be accepted is very strange, and apparently there is > only one program in existence that does. > > That a trailing '\0' was ever accepted was due to a bug in the code. > > Accepting '\0' in general in a text interface is a very dangerous and > buggy pattern so it must be done very carefully or else other > regressions or bugs could be easily introduced. Ok, Could you please give me more detail about the potential risk from the patch? What is the different behavior between the patch and old kernel? It seems like entirely same. > Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
According to POSIX standard, "The write() function shall attempt to write nbyte bytes from the buffer pointed to by buf to the file associated with the open file descriptor, fildes.". So it is not the length of string(strlen). On Mon, Sep 14, 2015 at 4:05 AM, Steven Rostedt wrote: > On Sun, 13 Sep 2015 20:39:31 +0800 > Sean Fu wrote: > > >> > Accepting a '\0' is not at all reasonable for a text interface. The >> > application that does it is buggy. >> It is hard to comprehend that the current kernel can accept two bytes >> "1 ", "1\t", "1\n" except "1\0". > > Um, it does not seem hard to comprehend at all. As this is a string, > and it acts the same as a printf() or strlen. > > strlen("1 ") == 2 > strlen("1\t") == 2 > strlen("1\n") == 2 > strlen("1\0") == 1 > > Big difference to me. > > -- Steve > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
According to POSIX standard, "The write() function shall attempt to write nbyte bytes from the buffer pointed to by buf to the file associated with the open file descriptor, fildes.". So it is not the length of string(strlen). On Mon, Sep 14, 2015 at 4:05 AM, Steven Rostedtwrote: > On Sun, 13 Sep 2015 20:39:31 +0800 > Sean Fu wrote: > > >> > Accepting a '\0' is not at all reasonable for a text interface. The >> > application that does it is buggy. >> It is hard to comprehend that the current kernel can accept two bytes >> "1 ", "1\t", "1\n" except "1\0". > > Um, it does not seem hard to comprehend at all. As this is a string, > and it acts the same as a printf() or strlen. > > strlen("1 ") == 2 > strlen("1\t") == 2 > strlen("1\n") == 2 > strlen("1\0") == 1 > > Big difference to me. > > -- Steve > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Mon, Sep 14, 2015 at 12:44 AM, Eric W. Biedermanwrote: > Sean Fu writes: > >> On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman >> wrote: >>> Sean Fu writes: > '\0' is not and has never been valid in a text file. > proc files are a text interface. > Expecting '\0' to be accepted is very strange, and apparently there is > only one program in existence that does. > > That a trailing '\0' was ever accepted was due to a bug in the code. > > Accepting '\0' in general in a text interface is a very dangerous and > buggy pattern so it must be done very carefully or else other > regressions or bugs could be easily introduced. Ok, Could you please give me more detail about the potential risk from the patch? What is the different behavior between the patch and old kernel? It seems like entirely same. > Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On September 15, 2015 4:30:56 AM CDT, Sean Fuwrote: >On Mon, Sep 14, 2015 at 12:44 AM, Eric W. Biederman > wrote: >> Sean Fu writes: >> >>> On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman >>> wrote: Sean Fu writes: >> '\0' is not and has never been valid in a text file. >> proc files are a text interface. >> Expecting '\0' to be accepted is very strange, and apparently there >is >> only one program in existence that does. >> >> That a trailing '\0' was ever accepted was due to a bug in the code. >> >> Accepting '\0' in general in a text interface is a very dangerous and >> buggy pattern so it must be done very carefully or else other >> regressions or bugs could be easily introduced. >Ok, >Could you please give me more detail about the potential risk from the >patch? Regressions. AKA There may now be programs that depend on writing a '\0' failing. >What is the different behavior between the patch and old kernel? >It seems like entirely same. Not at all. And clear explanations have already been given. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 15 Sep 2015 17:10:11 +0800 Sean Fuwrote: > According to POSIX standard, "The write() function shall attempt to > write nbyte bytes from the buffer pointed to by buf to the file > associated with the open file descriptor, fildes.". > So it is not the length of string(strlen). Why do we care about the "write()" function? Yeah, so be it. Send out as much data as you want. We care about the effects of the proc file system by the input it receives. You can send in X amounts of data. That's not broken. The only apps that would send out a string to the proc filesystem that includes "\0" and beyond is root kits. I'm sorry but your arguments are starting to go south. I'm starting to think that the app that broke is a root kit, and maybe it's best to just Nack this. -- Steve > > On Mon, Sep 14, 2015 at 4:05 AM, Steven Rostedt wrote: > > On Sun, 13 Sep 2015 20:39:31 +0800 > > Sean Fu wrote: > > > > > >> > Accepting a '\0' is not at all reasonable for a text interface. The > >> > application that does it is buggy. > >> It is hard to comprehend that the current kernel can accept two bytes > >> "1 ", "1\t", "1\n" except "1\0". > > > > Um, it does not seem hard to comprehend at all. As this is a string, > > and it acts the same as a printf() or strlen. > > > > strlen("1 ") == 2 > > strlen("1\t") == 2 > > strlen("1\n") == 2 > > strlen("1\0") == 1 > > > > Big difference to me. > > > > -- Steve > > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Sun, 13 Sep 2015 20:39:31 +0800 Sean Fu wrote: > > Accepting a '\0' is not at all reasonable for a text interface. The > > application that does it is buggy. > It is hard to comprehend that the current kernel can accept two bytes > "1 ", "1\t", "1\n" except "1\0". Um, it does not seem hard to comprehend at all. As this is a string, and it acts the same as a printf() or strlen. strlen("1 ") == 2 strlen("1\t") == 2 strlen("1\n") == 2 strlen("1\0") == 1 Big difference to me. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Sean Fu writes: > On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman > wrote: >> Sean Fu writes: >> Sounds like a reasonable compromise. Sean, can you make a patch that only affects the one proc file (comment it well in the code), and have it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it would only see "1 " >>> The current code uses uniform handler (e.g. "proc_dointvec") for all >>> same type proc file. >>> So all integer type proc file are affected. >> >> No. I do not believe the proprietary binary application you are dealing >> with writes to all proc files that use the proc_dointvec handler. > I means all ctl_table whose .proc_handler is "proc_dointvec" are > affected. I mean this only deserves consideration because this is a regression report. I mean by limiting this to only the proc files that are written to by the weird program that broke we can minimize the chances that anything else will break. I do not believe that the weird program that broke writes to every proc file. Certainly I have not heard that asserted. >>> In fact, The behavior of all integer type proc file should be changed. >> >> Not at all. The only files that we can possibly justify changing today >> are the files where an actual regression is being observed. >> >> Because quite frankly 5 years is way too long to wait to report a >> regression. By and large software is reasonable and treats proc >> files as text files where '\0' is an invalid character. > 5 years is not enough long for distros, specially enterprise distros. > The most of HuaWei machines run our SLES10sp3(2.6.16, SUSE LINUX > ENTERPRISE SERVER). > They use one enterprise version for 5+ years usually. If you want to play by enterprise kernel rules please talk to your enterprise kernel support people. >> Accepting a '\0' is not at all reasonable for a text interface. The >> application that does it is buggy. > It is hard to comprehend that the current kernel can accept two bytes > "1 ", "1\t", "1\n" except "1\0". '\0' is not and has never been valid in a text file. proc files are a text interface. Expecting '\0' to be accepted is very strange, and apparently there is only one program in existence that does. That a trailing '\0' was ever accepted was due to a bug in the code. Accepting '\0' in general in a text interface is a very dangerous and buggy pattern so it must be done very carefully or else other regressions or bugs could be easily introduced. That no one has complained about this in the 5 years since the change happened strongly indicates this no one else cares. A very targeted very narrow regression fix that only handles a trailing '\0' and that only changes the behavior of proc files that matter is reasonable. Or do you volunteer to go out and test every program that has been written or updated to write to proc in the last 5 years (since the behavior changed) and verify that none of them in no circumstances depend upon failing if an trailing '\0' is included? If you can audit all of the code written in the last 5 years and verify that the change will not introduce problems for any other user space program we can talk about changing all of the proc files that use proc_dointvec. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman wrote: > Sean Fu writes: > >>> Sounds like a reasonable compromise. Sean, can you make a patch that >>> only affects the one proc file (comment it well in the code), and have >>> it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it >>> would only see "1 " >> The current code uses uniform handler (e.g. "proc_dointvec") for all >> same type proc file. >> So all integer type proc file are affected. > > No. I do not believe the proprietary binary application you are dealing > with writes to all proc files that use the proc_dointvec handler. I means all ctl_table whose .proc_handler is "proc_dointvec" are affected. > >> In fact, The behavior of all integer type proc file should be changed. > > Not at all. The only files that we can possibly justify changing today > are the files where an actual regression is being observed. > > Because quite frankly 5 years is way too long to wait to report a > regression. By and large software is reasonable and treats proc > files as text files where '\0' is an invalid character. 5 years is not enough long for distros, specially enterprise distros. The most of HuaWei machines run our SLES10sp3(2.6.16, SUSE LINUX ENTERPRISE SERVER). They use one enterprise version for 5+ years usually. > > Accepting a '\0' is not at all reasonable for a text interface. The > application that does it is buggy. It is hard to comprehend that the current kernel can accept two bytes "1 ", "1\t", "1\n" except "1\0". > > Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biedermanwrote: > Sean Fu writes: > >>> Sounds like a reasonable compromise. Sean, can you make a patch that >>> only affects the one proc file (comment it well in the code), and have >>> it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it >>> would only see "1 " >> The current code uses uniform handler (e.g. "proc_dointvec") for all >> same type proc file. >> So all integer type proc file are affected. > > No. I do not believe the proprietary binary application you are dealing > with writes to all proc files that use the proc_dointvec handler. I means all ctl_table whose .proc_handler is "proc_dointvec" are affected. > >> In fact, The behavior of all integer type proc file should be changed. > > Not at all. The only files that we can possibly justify changing today > are the files where an actual regression is being observed. > > Because quite frankly 5 years is way too long to wait to report a > regression. By and large software is reasonable and treats proc > files as text files where '\0' is an invalid character. 5 years is not enough long for distros, specially enterprise distros. The most of HuaWei machines run our SLES10sp3(2.6.16, SUSE LINUX ENTERPRISE SERVER). They use one enterprise version for 5+ years usually. > > Accepting a '\0' is not at all reasonable for a text interface. The > application that does it is buggy. It is hard to comprehend that the current kernel can accept two bytes "1 ", "1\t", "1\n" except "1\0". > > Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Sean Fuwrites: > On Sat, Sep 12, 2015 at 1:01 AM, Eric W. Biederman > wrote: >> Sean Fu writes: >> Sounds like a reasonable compromise. Sean, can you make a patch that only affects the one proc file (comment it well in the code), and have it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it would only see "1 " >>> The current code uses uniform handler (e.g. "proc_dointvec") for all >>> same type proc file. >>> So all integer type proc file are affected. >> >> No. I do not believe the proprietary binary application you are dealing >> with writes to all proc files that use the proc_dointvec handler. > I means all ctl_table whose .proc_handler is "proc_dointvec" are > affected. I mean this only deserves consideration because this is a regression report. I mean by limiting this to only the proc files that are written to by the weird program that broke we can minimize the chances that anything else will break. I do not believe that the weird program that broke writes to every proc file. Certainly I have not heard that asserted. >>> In fact, The behavior of all integer type proc file should be changed. >> >> Not at all. The only files that we can possibly justify changing today >> are the files where an actual regression is being observed. >> >> Because quite frankly 5 years is way too long to wait to report a >> regression. By and large software is reasonable and treats proc >> files as text files where '\0' is an invalid character. > 5 years is not enough long for distros, specially enterprise distros. > The most of HuaWei machines run our SLES10sp3(2.6.16, SUSE LINUX > ENTERPRISE SERVER). > They use one enterprise version for 5+ years usually. If you want to play by enterprise kernel rules please talk to your enterprise kernel support people. >> Accepting a '\0' is not at all reasonable for a text interface. The >> application that does it is buggy. > It is hard to comprehend that the current kernel can accept two bytes > "1 ", "1\t", "1\n" except "1\0". '\0' is not and has never been valid in a text file. proc files are a text interface. Expecting '\0' to be accepted is very strange, and apparently there is only one program in existence that does. That a trailing '\0' was ever accepted was due to a bug in the code. Accepting '\0' in general in a text interface is a very dangerous and buggy pattern so it must be done very carefully or else other regressions or bugs could be easily introduced. That no one has complained about this in the 5 years since the change happened strongly indicates this no one else cares. A very targeted very narrow regression fix that only handles a trailing '\0' and that only changes the behavior of proc files that matter is reasonable. Or do you volunteer to go out and test every program that has been written or updated to write to proc in the last 5 years (since the behavior changed) and verify that none of them in no circumstances depend upon failing if an trailing '\0' is included? If you can audit all of the code written in the last 5 years and verify that the change will not introduce problems for any other user space program we can talk about changing all of the proc files that use proc_dointvec. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Sun, 13 Sep 2015 20:39:31 +0800 Sean Fuwrote: > > Accepting a '\0' is not at all reasonable for a text interface. The > > application that does it is buggy. > It is hard to comprehend that the current kernel can accept two bytes > "1 ", "1\t", "1\n" except "1\0". Um, it does not seem hard to comprehend at all. As this is a string, and it acts the same as a printf() or strlen. strlen("1 ") == 2 strlen("1\t") == 2 strlen("1\n") == 2 strlen("1\0") == 1 Big difference to me. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Sean Fu writes: >> Sounds like a reasonable compromise. Sean, can you make a patch that >> only affects the one proc file (comment it well in the code), and have >> it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it >> would only see "1 " > The current code uses uniform handler (e.g. "proc_dointvec") for all > same type proc file. > So all integer type proc file are affected. No. I do not believe the proprietary binary application you are dealing with writes to all proc files that use the proc_dointvec handler. > In fact, The behavior of all integer type proc file should be changed. Not at all. The only files that we can possibly justify changing today are the files where an actual regression is being observed. Because quite frankly 5 years is way too long to wait to report a regression. By and large software is reasonable and treats proc files as text files where '\0' is an invalid character. Accepting a '\0' is not at all reasonable for a text interface. The application that does it is buggy. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Fri, 11 Sep 2015 17:05:31 +0800 Sean Fu wrote: > > Sounds like a reasonable compromise. Sean, can you make a patch that > > only affects the one proc file (comment it well in the code), and have > > it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it > > would only see "1 " > The current code uses uniform handler (e.g. "proc_dointvec") for all > same type proc file. > So all integer type proc file are affected. > In fact, The behavior of all integer type proc file should be changed. Then at least make it where the \0 is the terminating string. Nothing after it will be seen by the rest of the code in the kernel. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, Sep 9, 2015 at 12:36 AM, Steven Rostedt wrote: > On Tue, 08 Sep 2015 11:19:14 -0500 > ebied...@xmission.com (Eric W. Biederman) wrote: > > >> This patch does not implement the old behavior. >> >> The old code does use '\0' as a buffer terminator, and because it does >> not check things closely I can see how it could accept a '\0' from >> userspace and treat that as an early buffer terminator. >> >> The patch treats '\0' as a number separator and allows things that have >> never been allowed before and quite frankly is very scary as it just >> invites bugs. >> >> So I do not think we should merge the given patch. It is just wrong. >> One that simply truncates the input buffer at the first '\0' character I >> think we can consider, although I am not a fan. > > I agree, and was thinking this patch did that, but I didn't look close > enough (why I never gave a Reviewed-by to it either). > > >> >> Steve as far as what I think would break. I don't think the current >> behavior should have broken anything and apparently it did. I don't see >> what a change that simply truncates the buffer at the first embedded >> '\0' would break, but I don't know how to test that there isn't anything >> that it will. We are way past the point of reasonable expectations >> being able to guide us. 4 years should have been more than enough soak >> time to have been able to say that the change was good, but apparently >> it was not. > > Well, to be fair, a lot of people (distros, etc) do not use the most > recent kernels. 4 years may be the first time a tool touches a kernel. > >> >> My gut feel says that if we are going to change this, at this late date, >> we find the one specific proc file that matters and change it just for >> that one proc file, and in that change we treat '\0' as a terminator not >> as a separator. I never did see in the conversation which proc file it >> is that actually matters. The principle is that the more precise and >> the more localized such a change is the less chance it has of causing a >> regression of something else, and the greater the chance we can look at >> a specific issue. > > Sounds like a reasonable compromise. Sean, can you make a patch that > only affects the one proc file (comment it well in the code), and have > it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it > would only see "1 " The current code uses uniform handler (e.g. "proc_dointvec") for all same type proc file. So all integer type proc file are affected. In fact, The behavior of all integer type proc file should be changed. > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Sean Fuwrites: >> Sounds like a reasonable compromise. Sean, can you make a patch that >> only affects the one proc file (comment it well in the code), and have >> it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it >> would only see "1 " > The current code uses uniform handler (e.g. "proc_dointvec") for all > same type proc file. > So all integer type proc file are affected. No. I do not believe the proprietary binary application you are dealing with writes to all proc files that use the proc_dointvec handler. > In fact, The behavior of all integer type proc file should be changed. Not at all. The only files that we can possibly justify changing today are the files where an actual regression is being observed. Because quite frankly 5 years is way too long to wait to report a regression. By and large software is reasonable and treats proc files as text files where '\0' is an invalid character. Accepting a '\0' is not at all reasonable for a text interface. The application that does it is buggy. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, Sep 9, 2015 at 12:36 AM, Steven Rostedtwrote: > On Tue, 08 Sep 2015 11:19:14 -0500 > ebied...@xmission.com (Eric W. Biederman) wrote: > > >> This patch does not implement the old behavior. >> >> The old code does use '\0' as a buffer terminator, and because it does >> not check things closely I can see how it could accept a '\0' from >> userspace and treat that as an early buffer terminator. >> >> The patch treats '\0' as a number separator and allows things that have >> never been allowed before and quite frankly is very scary as it just >> invites bugs. >> >> So I do not think we should merge the given patch. It is just wrong. >> One that simply truncates the input buffer at the first '\0' character I >> think we can consider, although I am not a fan. > > I agree, and was thinking this patch did that, but I didn't look close > enough (why I never gave a Reviewed-by to it either). > > >> >> Steve as far as what I think would break. I don't think the current >> behavior should have broken anything and apparently it did. I don't see >> what a change that simply truncates the buffer at the first embedded >> '\0' would break, but I don't know how to test that there isn't anything >> that it will. We are way past the point of reasonable expectations >> being able to guide us. 4 years should have been more than enough soak >> time to have been able to say that the change was good, but apparently >> it was not. > > Well, to be fair, a lot of people (distros, etc) do not use the most > recent kernels. 4 years may be the first time a tool touches a kernel. > >> >> My gut feel says that if we are going to change this, at this late date, >> we find the one specific proc file that matters and change it just for >> that one proc file, and in that change we treat '\0' as a terminator not >> as a separator. I never did see in the conversation which proc file it >> is that actually matters. The principle is that the more precise and >> the more localized such a change is the less chance it has of causing a >> regression of something else, and the greater the chance we can look at >> a specific issue. > > Sounds like a reasonable compromise. Sean, can you make a patch that > only affects the one proc file (comment it well in the code), and have > it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it > would only see "1 " The current code uses uniform handler (e.g. "proc_dointvec") for all same type proc file. So all integer type proc file are affected. In fact, The behavior of all integer type proc file should be changed. > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Fri, 11 Sep 2015 17:05:31 +0800 Sean Fuwrote: > > Sounds like a reasonable compromise. Sean, can you make a patch that > > only affects the one proc file (comment it well in the code), and have > > it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it > > would only see "1 " > The current code uses uniform handler (e.g. "proc_dointvec") for all > same type proc file. > So all integer type proc file are affected. > In fact, The behavior of all integer type proc file should be changed. Then at least make it where the \0 is the terminating string. Nothing after it will be seen by the rest of the code in the kernel. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 08 Sep 2015 11:19:14 -0500 ebied...@xmission.com (Eric W. Biederman) wrote: > This patch does not implement the old behavior. > > The old code does use '\0' as a buffer terminator, and because it does > not check things closely I can see how it could accept a '\0' from > userspace and treat that as an early buffer terminator. > > The patch treats '\0' as a number separator and allows things that have > never been allowed before and quite frankly is very scary as it just > invites bugs. > > So I do not think we should merge the given patch. It is just wrong. > One that simply truncates the input buffer at the first '\0' character I > think we can consider, although I am not a fan. I agree, and was thinking this patch did that, but I didn't look close enough (why I never gave a Reviewed-by to it either). > > Steve as far as what I think would break. I don't think the current > behavior should have broken anything and apparently it did. I don't see > what a change that simply truncates the buffer at the first embedded > '\0' would break, but I don't know how to test that there isn't anything > that it will. We are way past the point of reasonable expectations > being able to guide us. 4 years should have been more than enough soak > time to have been able to say that the change was good, but apparently > it was not. Well, to be fair, a lot of people (distros, etc) do not use the most recent kernels. 4 years may be the first time a tool touches a kernel. > > My gut feel says that if we are going to change this, at this late date, > we find the one specific proc file that matters and change it just for > that one proc file, and in that change we treat '\0' as a terminator not > as a separator. I never did see in the conversation which proc file it > is that actually matters. The principle is that the more precise and > the more localized such a change is the less chance it has of causing a > regression of something else, and the greater the chance we can look at > a specific issue. Sounds like a reasonable compromise. Sean, can you make a patch that only affects the one proc file (comment it well in the code), and have it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it would only see "1 " -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Steven Rostedt writes: > On Tue, 8 Sep 2015 11:11:38 +0800 > Sean Fu wrote: > >> On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: >> > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: >> >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt >> >> wrote: >> >>> On Thu, 27 Aug 2015 08:17:29 +0800 >> >>> Sean Fu wrote: >> strace execute result: >> write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) >> > If vleft > 1, "1\0 2" is treated as invalid paraments and all string >> > include '\0' will be invalid. >> Hi All experts, >> Could you please signed off this patch? > > If anyone should take this, it would be Andrew. > > I have no issue with the patch. Eric, you had some issue, but I don't > see a scenario that would depend on the current behavior. That is, what > do you think would break if we put it back to the old behavior? This patch does not implement the old behavior. The old code does use '\0' as a buffer terminator, and because it does not check things closely I can see how it could accept a '\0' from userspace and treat that as an early buffer terminator. The patch treats '\0' as a number separator and allows things that have never been allowed before and quite frankly is very scary as it just invites bugs. So I do not think we should merge the given patch. It is just wrong. One that simply truncates the input buffer at the first '\0' character I think we can consider, although I am not a fan. Steve as far as what I think would break. I don't think the current behavior should have broken anything and apparently it did. I don't see what a change that simply truncates the buffer at the first embedded '\0' would break, but I don't know how to test that there isn't anything that it will. We are way past the point of reasonable expectations being able to guide us. 4 years should have been more than enough soak time to have been able to say that the change was good, but apparently it was not. My gut feel says that if we are going to change this, at this late date, we find the one specific proc file that matters and change it just for that one proc file, and in that change we treat '\0' as a terminator not as a separator. I never did see in the conversation which proc file it is that actually matters. The principle is that the more precise and the more localized such a change is the less chance it has of causing a regression of something else, and the greater the chance we can look at a specific issue. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 8 Sep 2015 11:11:38 +0800 Sean Fu wrote: > On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: > > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: > >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt > >> wrote: > >>> On Thu, 27 Aug 2015 08:17:29 +0800 > >>> Sean Fu wrote: > strace execute result: > write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > > If vleft > 1, "1\0 2" is treated as invalid paraments and all string > > include '\0' will be invalid. > Hi All experts, > Could you please signed off this patch? If anyone should take this, it would be Andrew. I have no issue with the patch. Eric, you had some issue, but I don't see a scenario that would depend on the current behavior. That is, what do you think would break if we put it back to the old behavior? -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 8 Sep 2015 11:11:38 +0800 Sean Fuwrote: > On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: > > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: > >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt > >> wrote: > >>> On Thu, 27 Aug 2015 08:17:29 +0800 > >>> Sean Fu wrote: > strace execute result: > write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > > If vleft > 1, "1\0 2" is treated as invalid paraments and all string > > include '\0' will be invalid. > Hi All experts, > Could you please signed off this patch? If anyone should take this, it would be Andrew. I have no issue with the patch. Eric, you had some issue, but I don't see a scenario that would depend on the current behavior. That is, what do you think would break if we put it back to the old behavior? -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Steven Rostedtwrites: > On Tue, 8 Sep 2015 11:11:38 +0800 > Sean Fu wrote: > >> On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: >> > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: >> >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt >> >> wrote: >> >>> On Thu, 27 Aug 2015 08:17:29 +0800 >> >>> Sean Fu wrote: >> strace execute result: >> write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) >> > If vleft > 1, "1\0 2" is treated as invalid paraments and all string >> > include '\0' will be invalid. >> Hi All experts, >> Could you please signed off this patch? > > If anyone should take this, it would be Andrew. > > I have no issue with the patch. Eric, you had some issue, but I don't > see a scenario that would depend on the current behavior. That is, what > do you think would break if we put it back to the old behavior? This patch does not implement the old behavior. The old code does use '\0' as a buffer terminator, and because it does not check things closely I can see how it could accept a '\0' from userspace and treat that as an early buffer terminator. The patch treats '\0' as a number separator and allows things that have never been allowed before and quite frankly is very scary as it just invites bugs. So I do not think we should merge the given patch. It is just wrong. One that simply truncates the input buffer at the first '\0' character I think we can consider, although I am not a fan. Steve as far as what I think would break. I don't think the current behavior should have broken anything and apparently it did. I don't see what a change that simply truncates the buffer at the first embedded '\0' would break, but I don't know how to test that there isn't anything that it will. We are way past the point of reasonable expectations being able to guide us. 4 years should have been more than enough soak time to have been able to say that the change was good, but apparently it was not. My gut feel says that if we are going to change this, at this late date, we find the one specific proc file that matters and change it just for that one proc file, and in that change we treat '\0' as a terminator not as a separator. I never did see in the conversation which proc file it is that actually matters. The principle is that the more precise and the more localized such a change is the less chance it has of causing a regression of something else, and the greater the chance we can look at a specific issue. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 08 Sep 2015 11:19:14 -0500 ebied...@xmission.com (Eric W. Biederman) wrote: > This patch does not implement the old behavior. > > The old code does use '\0' as a buffer terminator, and because it does > not check things closely I can see how it could accept a '\0' from > userspace and treat that as an early buffer terminator. > > The patch treats '\0' as a number separator and allows things that have > never been allowed before and quite frankly is very scary as it just > invites bugs. > > So I do not think we should merge the given patch. It is just wrong. > One that simply truncates the input buffer at the first '\0' character I > think we can consider, although I am not a fan. I agree, and was thinking this patch did that, but I didn't look close enough (why I never gave a Reviewed-by to it either). > > Steve as far as what I think would break. I don't think the current > behavior should have broken anything and apparently it did. I don't see > what a change that simply truncates the buffer at the first embedded > '\0' would break, but I don't know how to test that there isn't anything > that it will. We are way past the point of reasonable expectations > being able to guide us. 4 years should have been more than enough soak > time to have been able to say that the change was good, but apparently > it was not. Well, to be fair, a lot of people (distros, etc) do not use the most recent kernels. 4 years may be the first time a tool touches a kernel. > > My gut feel says that if we are going to change this, at this late date, > we find the one specific proc file that matters and change it just for > that one proc file, and in that change we treat '\0' as a terminator not > as a separator. I never did see in the conversation which proc file it > is that actually matters. The principle is that the more precise and > the more localized such a change is the less chance it has of causing a > regression of something else, and the greater the chance we can look at > a specific issue. Sounds like a reasonable compromise. Sean, can you make a patch that only affects the one proc file (comment it well in the code), and have it accept nothing past the '\0'. Even if someone passed in "1 \0 2", it would only see "1 " -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Fri, Aug 28, 2015 at 11:31 AM, Sean Fu wrote: > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt wrote: >>> On Thu, 27 Aug 2015 08:17:29 +0800 >>> Sean Fu wrote: strace execute result: write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > If vleft > 1, "1\0 2" is treated as invalid paraments and all string > include '\0' will be invalid. Hi All experts, Could you please signed off this patch? > > > -- Steve >>> -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Fri, Aug 28, 2015 at 11:31 AM, Sean Fuwrote: > On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: >> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt wrote: >>> On Thu, 27 Aug 2015 08:17:29 +0800 >>> Sean Fu wrote: strace execute result: write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > If vleft > 1, "1\0 2" is treated as invalid paraments and all string > include '\0' will be invalid. Hi All experts, Could you please signed off this patch? > > > -- Steve >>> -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu wrote: > On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt wrote: >> On Thu, 27 Aug 2015 08:17:29 +0800 >> Sean Fu wrote: >> >>> On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt wrote: >>> > On Wed, 26 Aug 2015 23:48:01 +0800 >>> > Sean Fu wrote: >>> > >>> > >>> >> > Defending the patch, I can't imagine any user space code expecting the >>> >> > current behavior. The current behavior is that if you write "1\0" it >>> >> > will error out instead of accepting the "1". I can't come up with a >>> >> > scenario that would require userspace to expect "1\0" to fail. Can you? >>> >> Thanks >>> > >>> > Although, with the current patch, would "1\02" fail? It should. >>> Yes, "1\02" is equal to "1\2"(count=2) or "1\2\0"(count=3), So it should >>> fail. >> >> Sorry, I meant "1\0 2" > In this case, The patch behavior is accepting the "1" and discarding > other bytes. > for (; left && vleft--; i++, first=0) {//vleft is 1 for integer > type or unsigned long type proc file > >> >> -- Steve >> >>> >>> code >>> len = write(fd, "1\0\2", 3); >>> >>> strace execute result: >>> write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) If vleft > 1, "1\0 2" is treated as invalid paraments and all string include '\0' will be invalid. >>> >>> > >>> > -- Steve >> -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt wrote: > On Thu, 27 Aug 2015 08:17:29 +0800 > Sean Fu wrote: > >> On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt wrote: >> > On Wed, 26 Aug 2015 23:48:01 +0800 >> > Sean Fu wrote: >> > >> > >> >> > Defending the patch, I can't imagine any user space code expecting the >> >> > current behavior. The current behavior is that if you write "1\0" it >> >> > will error out instead of accepting the "1". I can't come up with a >> >> > scenario that would require userspace to expect "1\0" to fail. Can you? >> >> Thanks >> > >> > Although, with the current patch, would "1\02" fail? It should. >> Yes, "1\02" is equal to "1\2"(count=2) or "1\2\0"(count=3), So it should >> fail. > > Sorry, I meant "1\0 2" In this case, The patch behavior is accepting the "1" and discarding other bytes. for (; left && vleft--; i++, first=0) {//vleft is 1 for integer type or unsigned long type proc file > > -- Steve > >> >> code >> len = write(fd, "1\0\2", 3); >> >> strace execute result: >> write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) >> >> > >> > -- Steve > -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 4:32 PM, Sean Fu fxinr...@gmail.com wrote: On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt rost...@goodmis.org wrote: On Thu, 27 Aug 2015 08:17:29 +0800 Sean Fu fxinr...@gmail.com wrote: On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt rost...@goodmis.org wrote: On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu fxinr...@gmail.com wrote: Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks Although, with the current patch, would 1\02 fail? It should. Yes, 1\02 is equal to 1\2(count=2) or 1\2\0(count=3), So it should fail. Sorry, I meant 1\0 2 In this case, The patch behavior is accepting the 1 and discarding other bytes. for (; left vleft--; i++, first=0) {//vleft is 1 for integer type or unsigned long type proc file -- Steve code len = write(fd, 1\0\2, 3); strace execute result: write(3, 1\2\0, 3)= -1 EINVAL (Invalid argument) If vleft 1, 1\0 2 is treated as invalid paraments and all string include '\0' will be invalid. -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt rost...@goodmis.org wrote: On Thu, 27 Aug 2015 08:17:29 +0800 Sean Fu fxinr...@gmail.com wrote: On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt rost...@goodmis.org wrote: On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu fxinr...@gmail.com wrote: Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks Although, with the current patch, would 1\02 fail? It should. Yes, 1\02 is equal to 1\2(count=2) or 1\2\0(count=3), So it should fail. Sorry, I meant 1\0 2 In this case, The patch behavior is accepting the 1 and discarding other bytes. for (; left vleft--; i++, first=0) {//vleft is 1 for integer type or unsigned long type proc file -- Steve code len = write(fd, 1\0\2, 3); strace execute result: write(3, 1\2\0, 3)= -1 EINVAL (Invalid argument) -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Thu, 27 Aug 2015 08:17:29 +0800 Sean Fu wrote: > On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt wrote: > > On Wed, 26 Aug 2015 23:48:01 +0800 > > Sean Fu wrote: > > > > > >> > Defending the patch, I can't imagine any user space code expecting the > >> > current behavior. The current behavior is that if you write "1\0" it > >> > will error out instead of accepting the "1". I can't come up with a > >> > scenario that would require userspace to expect "1\0" to fail. Can you? > >> Thanks > > > > Although, with the current patch, would "1\02" fail? It should. > Yes, "1\02" is equal to "1\2"(count=2) or "1\2\0"(count=3), So it should fail. Sorry, I meant "1\0 2" -- Steve > > code > len = write(fd, "1\0\2", 3); > > strace execute result: > write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > > > > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 4:39 AM, Heinrich Schuchardt wrote: > > > On 24.08.2015 10:56, Sean Fu wrote: >> when the input argument "count" including the terminating byte "\0", >> The write system call return EINVAL on proc file. >> But it return success on regular file. >> >> E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". >> write(fd, "1\0", 2) return EINVAL. > > Reading through kernel/sysctl.c it looks like you are allowing > "1\01" to be used to pass two integers or two longs. > This is not what you describe as target of your patch. "1\01" actually is "1\1", So either of them will fail. > > Parameter tr returned from proc_get_long should be checked in > __do_proc_dointvec, > __do_proc_doulongvec_minmax. > > Best regards > > Heinrich Schuchardt > >> >> Signed-off-by: Sean Fu >> --- >> kernel/sysctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 19b62b5..c2b0594 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, >> unsigned long *lvalp, >> return 0; >> } >> >> -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; >> +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; >> >> static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, >> int write, void __user *buffer, >> -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt wrote: > On Wed, 26 Aug 2015 23:48:01 +0800 > Sean Fu wrote: > > >> > Defending the patch, I can't imagine any user space code expecting the >> > current behavior. The current behavior is that if you write "1\0" it >> > will error out instead of accepting the "1". I can't come up with a >> > scenario that would require userspace to expect "1\0" to fail. Can you? >> Thanks > > Although, with the current patch, would "1\02" fail? It should. Yes, "1\02" is equal to "1\2"(count=2) or "1\2\0"(count=3), So it should fail. code len = write(fd, "1\0\2", 3); strace execute result: write(3, "1\2\0", 3)= -1 EINVAL (Invalid argument) > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu wrote: > > Defending the patch, I can't imagine any user space code expecting the > > current behavior. The current behavior is that if you write "1\0" it > > will error out instead of accepting the "1". I can't come up with a > > scenario that would require userspace to expect "1\0" to fail. Can you? > Thanks Although, with the current patch, would "1\02" fail? It should. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 3:05 AM, Steven Rostedt wrote: > On Tue, 25 Aug 2015 13:33:57 -0400 > Austin S Hemmelgarn wrote: > >> >> How do you know that? >> > I will prove that all other write usage is not impacted later. >> Except that you can only really do this for programs that you have >> access to, and by definition you can not have access to every program >> ever written that writes to /proc. >> >> If you were going to do this, it would need to be itself controlled by >> another sysctl to toggle the behavior, which would need to default to >> the current behavior. > > Defending the patch, I can't imagine any user space code expecting the > current behavior. The current behavior is that if you write "1\0" it > will error out instead of accepting the "1". I can't come up with a > scenario that would require userspace to expect "1\0" to fail. Can you? Thanks > > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 4:39 AM, Heinrich Schuchardt wrote: > > > On 24.08.2015 10:56, Sean Fu wrote: >> when the input argument "count" including the terminating byte "\0", >> The write system call return EINVAL on proc file. >> But it return success on regular file. >> >> E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". >> write(fd, "1\0", 2) return EINVAL. > > Reading through kernel/sysctl.c it looks like you are allowing > "1\01" to be used to pass two integers or two longs. > This is not what you describe as target of your patch. 1st 2nd 3rd Change? '0'~'9' '\0' non '\0'No proc_get_long-->simple_strtoul-->simple_strtoull-->_parse_integer __do_proc_dointvec ... vleft = table->maxlen / sizeof(*i); //vleft = 1 if it is integer type proc file ... for (; left && vleft--; i++, first=0) { //In last loop left=2, but vleft = 0 cause exit. > > Parameter tr returned from proc_get_long should be checked in > __do_proc_dointvec, > __do_proc_doulongvec_minmax. > > Best regards > > Heinrich Schuchardt > >> >> Signed-off-by: Sean Fu >> --- >> kernel/sysctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/kernel/sysctl.c b/kernel/sysctl.c >> index 19b62b5..c2b0594 100644 >> --- a/kernel/sysctl.c >> +++ b/kernel/sysctl.c >> @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, >> unsigned long *lvalp, >> return 0; >> } >> >> -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; >> +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; >> >> static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, >> int write, void __user *buffer, >> All possibilities are listed. 1 byte data(count = 1) 1st Change? '\0' NO non '\0'NO 2 bytes data(count = 2) 1st 2nd Change? '0'~'9' '\0' Yes '0'~'9' non '\0'No non number'\0' No non numbernon '\0' No 3 bytes data(count = 3) 1st 2nd 3rd Change? '0'~'9' '0'~'9''\0' Yes '0'~'9' '0'~'9'non '\0' No '0'~'9' non '0'~'9' '\0' No '0'~'9' non '0'~'9' non '\0' No '0'~'9' '\0''\0' No '0'~'9' '\0' non '\0'No non '0'~'9' Any AnyNo More 3 bytes data(count > 3) Number sequence the next character Change? "x1...xn" '\0' Yes "x1...xn" non '\0' No Non "x1...xn" '\0' No Non "x1...xn" non '\0'No "x1...xn" is a string whose all members are "0"~'9' Non "x1...xn" means the first character is not "0"~'9'. "Yes" means the behavior is changed. "No" means the behavior is Not changed. -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 3:05 AM, Steven Rostedt rost...@goodmis.org wrote: On Tue, 25 Aug 2015 13:33:57 -0400 Austin S Hemmelgarn ahferro...@gmail.com wrote: How do you know that? I will prove that all other write usage is not impacted later. Except that you can only really do this for programs that you have access to, and by definition you can not have access to every program ever written that writes to /proc. If you were going to do this, it would need to be itself controlled by another sysctl to toggle the behavior, which would need to default to the current behavior. Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu fxinr...@gmail.com wrote: Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks Although, with the current patch, would 1\02 fail? It should. -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 4:39 AM, Heinrich Schuchardt xypron.g...@gmx.de wrote: On 24.08.2015 10:56, Sean Fu wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Reading through kernel/sysctl.c it looks like you are allowing 1\01 to be used to pass two integers or two longs. This is not what you describe as target of your patch. 1st 2nd 3rd Change? '0'~'9' '\0' non '\0'No proc_get_long--simple_strtoul--simple_strtoull--_parse_integer __do_proc_dointvec ... vleft = table-maxlen / sizeof(*i); //vleft = 1 if it is integer type proc file ... for (; left vleft--; i++, first=0) { //In last loop left=2, but vleft = 0 cause exit. Parameter tr returned from proc_get_long should be checked in __do_proc_dointvec, __do_proc_doulongvec_minmax. Best regards Heinrich Schuchardt Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, All possibilities are listed. 1 byte data(count = 1) 1st Change? '\0' NO non '\0'NO 2 bytes data(count = 2) 1st 2nd Change? '0'~'9' '\0' Yes '0'~'9' non '\0'No non number'\0' No non numbernon '\0' No 3 bytes data(count = 3) 1st 2nd 3rd Change? '0'~'9' '0'~'9''\0' Yes '0'~'9' '0'~'9'non '\0' No '0'~'9' non '0'~'9' '\0' No '0'~'9' non '0'~'9' non '\0' No '0'~'9' '\0''\0' No '0'~'9' '\0' non '\0'No non '0'~'9' Any AnyNo More 3 bytes data(count 3) Number sequence the next character Change? x1...xn '\0' Yes x1...xn non '\0' No Non x1...xn '\0' No Non x1...xn non '\0'No x1...xn is a string whose all members are 0~'9' Non x1...xn means the first character is not 0~'9'. Yes means the behavior is changed. No means the behavior is Not changed. -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Wed, Aug 26, 2015 at 4:39 AM, Heinrich Schuchardt xypron.g...@gmx.de wrote: On 24.08.2015 10:56, Sean Fu wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Reading through kernel/sysctl.c it looks like you are allowing 1\01 to be used to pass two integers or two longs. This is not what you describe as target of your patch. 1\01 actually is 1\1, So either of them will fail. Parameter tr returned from proc_get_long should be checked in __do_proc_dointvec, __do_proc_doulongvec_minmax. Best regards Heinrich Schuchardt Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt rost...@goodmis.org wrote: On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu fxinr...@gmail.com wrote: Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks Although, with the current patch, would 1\02 fail? It should. Yes, 1\02 is equal to 1\2(count=2) or 1\2\0(count=3), So it should fail. code len = write(fd, 1\0\2, 3); strace execute result: write(3, 1\2\0, 3)= -1 EINVAL (Invalid argument) -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Thu, 27 Aug 2015 08:17:29 +0800 Sean Fu fxinr...@gmail.com wrote: On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt rost...@goodmis.org wrote: On Wed, 26 Aug 2015 23:48:01 +0800 Sean Fu fxinr...@gmail.com wrote: Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? Thanks Although, with the current patch, would 1\02 fail? It should. Yes, 1\02 is equal to 1\2(count=2) or 1\2\0(count=3), So it should fail. Sorry, I meant 1\0 2 -- Steve code len = write(fd, 1\0\2, 3); strace execute result: write(3, 1\2\0, 3)= -1 EINVAL (Invalid argument) -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On 24.08.2015 10:56, Sean Fu wrote: > when the input argument "count" including the terminating byte "\0", > The write system call return EINVAL on proc file. > But it return success on regular file. > > E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". > write(fd, "1\0", 2) return EINVAL. Reading through kernel/sysctl.c it looks like you are allowing "1\01" to be used to pass two integers or two longs. This is not what you describe as target of your patch. Parameter tr returned from proc_get_long should be checked in __do_proc_dointvec, __do_proc_doulongvec_minmax. Best regards Heinrich Schuchardt > > Signed-off-by: Sean Fu > --- > kernel/sysctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 19b62b5..c2b0594 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, > unsigned long *lvalp, > return 0; > } > > -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; > +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; > > static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, > int write, void __user *buffer, > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 25 Aug 2015 13:33:57 -0400 Austin S Hemmelgarn wrote: > >> How do you know that? > > I will prove that all other write usage is not impacted later. > Except that you can only really do this for programs that you have > access to, and by definition you can not have access to every program > ever written that writes to /proc. > > If you were going to do this, it would need to be itself controlled by > another sysctl to toggle the behavior, which would need to default to > the current behavior. Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write "1\0" it will error out instead of accepting the "1". I can't come up with a scenario that would require userspace to expect "1\0" to fail. Can you? -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On 2015-08-25 12:44, Sean Fu wrote: On Tue, Aug 25, 2015 at 10:15 PM, Steven Rostedt wrote: On Tue, 25 Aug 2015 15:50:18 +0800 Sean Fu wrote: On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman wrote: On August 24, 2015 6:57:57 PM MDT, Sean Fu wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. The thing is, the above argument is against the patch. The behavior changed 2 years ago, and nobody noticed. Changing it back only causes more inconsistent behavior. It is impossible to cause more inconsistent behavior. it just enhance compatibility(support "xx...x\0"). This patch just modify "proc_wspace_sep" array. and "proc_wspace_sep" is static. Only "proc_get_long" used this array, "proc_get_long" is also static. There are only 4 place to call "proc_get_long" in kernel/sysctl.c. I will prove that these 4 callers have no bad impact later. Except that it is userspace we're worrying about here, not stuff internal to the kernel. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. Writing to a proc file causes kernel actions. Writing to a regular file just saves data. That's not an argument here. 3 This patch does not have any potential compatibility risk with 3rd party application. How do you know that? I will prove that all other write usage is not impacted later. Except that you can only really do this for programs that you have access to, and by definition you can not have access to every program ever written that writes to /proc. If you were going to do this, it would need to be itself controlled by another sysctl to toggle the behavior, which would need to default to the current behavior. Thanks for all reply. Sean -- Steve 4 Support writting "1...\0" to proc file. Eric -- 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/ smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, Aug 25, 2015 at 10:15 PM, Steven Rostedt wrote: > On Tue, 25 Aug 2015 15:50:18 +0800 > Sean Fu wrote: > >> On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman >> wrote: >> > >> > >> > On August 24, 2015 6:57:57 PM MDT, Sean Fu wrote: >> >>An application from HuaWei which works fine on 2.6 encounters this >> >>issue on 3.0 or later kernel. >> > >> > My sympathies. Being stuck with a 3rd party application you can barely >> > talk about that has been broken for 5years and no one reported it. >> > >> > Ordinarily we would fix a regression like this. As it has been 5years the >> > challenge now is how do we tell if there are applications that depend on >> > the current behavior. >> > >> > Before we can change the behavior back we need a convincing argument that >> > we won't cause a regression in another application by making the change. >> > >> > I do not see how such an argument can be made. So you have my sympathies >> > but I do not see how we can help you. >> We should consider this patch basing on my following arguments. >> 1 Different version kernel should keep consistent on this behavior. > > The thing is, the above argument is against the patch. The behavior > changed 2 years ago, and nobody noticed. Changing it back only causes > more inconsistent behavior. It is impossible to cause more inconsistent behavior. it just enhance compatibility(support "xx...x\0"). This patch just modify "proc_wspace_sep" array. and "proc_wspace_sep" is static. Only "proc_get_long" used this array, "proc_get_long" is also static. There are only 4 place to call "proc_get_long" in kernel/sysctl.c. I will prove that these 4 callers have no bad impact later. > > >> 2 This writting behavior on proc file should be same with writting on >> regular file as possible as we can. > > Writing to a proc file causes kernel actions. Writing to a regular file > just saves data. That's not an argument here. > >> 3 This patch does not have any potential compatibility risk with 3rd >> party application. > > How do you know that? I will prove that all other write usage is not impacted later. Thanks for all reply. Sean > > -- Steve > >> 4 Support writting "1...\0" to proc file. >> >> > >> > Eric >> > > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, 25 Aug 2015 15:50:18 +0800 Sean Fu wrote: > On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman > wrote: > > > > > > On August 24, 2015 6:57:57 PM MDT, Sean Fu wrote: > >>An application from HuaWei which works fine on 2.6 encounters this > >>issue on 3.0 or later kernel. > > > > My sympathies. Being stuck with a 3rd party application you can barely > > talk about that has been broken for 5years and no one reported it. > > > > Ordinarily we would fix a regression like this. As it has been 5years the > > challenge now is how do we tell if there are applications that depend on > > the current behavior. > > > > Before we can change the behavior back we need a convincing argument that > > we won't cause a regression in another application by making the change. > > > > I do not see how such an argument can be made. So you have my sympathies > > but I do not see how we can help you. > We should consider this patch basing on my following arguments. > 1 Different version kernel should keep consistent on this behavior. The thing is, the above argument is against the patch. The behavior changed 2 years ago, and nobody noticed. Changing it back only causes more inconsistent behavior. > 2 This writting behavior on proc file should be same with writting on > regular file as possible as we can. Writing to a proc file causes kernel actions. Writing to a regular file just saves data. That's not an argument here. > 3 This patch does not have any potential compatibility risk with 3rd > party application. How do you know that? -- Steve > 4 Support writting "1...\0" to proc file. > > > > > Eric > > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman wrote: > > > On August 24, 2015 6:57:57 PM MDT, Sean Fu wrote: >>An application from HuaWei which works fine on 2.6 encounters this >>issue on 3.0 or later kernel. > > My sympathies. Being stuck with a 3rd party application you can barely talk > about that has been broken for 5years and no one reported it. > > Ordinarily we would fix a regression like this. As it has been 5years the > challenge now is how do we tell if there are applications that depend on the > current behavior. > > Before we can change the behavior back we need a convincing argument that we > won't cause a regression in another application by making the change. > > I do not see how such an argument can be made. So you have my sympathies but > I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. 3 This patch does not have any potential compatibility risk with 3rd party application. 4 Support writting "1...\0" to proc file. > > Eric > -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On 2015-08-25 12:44, Sean Fu wrote: On Tue, Aug 25, 2015 at 10:15 PM, Steven Rostedt rost...@goodmis.org wrote: On Tue, 25 Aug 2015 15:50:18 +0800 Sean Fu fxinr...@gmail.com wrote: On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 6:57:57 PM MDT, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. The thing is, the above argument is against the patch. The behavior changed 2 years ago, and nobody noticed. Changing it back only causes more inconsistent behavior. It is impossible to cause more inconsistent behavior. it just enhance compatibility(support xx...x\0). This patch just modify proc_wspace_sep array. and proc_wspace_sep is static. Only proc_get_long used this array, proc_get_long is also static. There are only 4 place to call proc_get_long in kernel/sysctl.c. I will prove that these 4 callers have no bad impact later. Except that it is userspace we're worrying about here, not stuff internal to the kernel. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. Writing to a proc file causes kernel actions. Writing to a regular file just saves data. That's not an argument here. 3 This patch does not have any potential compatibility risk with 3rd party application. How do you know that? I will prove that all other write usage is not impacted later. Except that you can only really do this for programs that you have access to, and by definition you can not have access to every program ever written that writes to /proc. If you were going to do this, it would need to be itself controlled by another sysctl to toggle the behavior, which would need to default to the current behavior. Thanks for all reply. Sean -- Steve 4 Support writting 1...\0 to proc file. Eric -- 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/ smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Tue, Aug 25, 2015 at 10:15 PM, Steven Rostedt rost...@goodmis.org wrote: On Tue, 25 Aug 2015 15:50:18 +0800 Sean Fu fxinr...@gmail.com wrote: On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 6:57:57 PM MDT, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. The thing is, the above argument is against the patch. The behavior changed 2 years ago, and nobody noticed. Changing it back only causes more inconsistent behavior. It is impossible to cause more inconsistent behavior. it just enhance compatibility(support xx...x\0). This patch just modify proc_wspace_sep array. and proc_wspace_sep is static. Only proc_get_long used this array, proc_get_long is also static. There are only 4 place to call proc_get_long in kernel/sysctl.c. I will prove that these 4 callers have no bad impact later. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. Writing to a proc file causes kernel actions. Writing to a regular file just saves data. That's not an argument here. 3 This patch does not have any potential compatibility risk with 3rd party application. How do you know that? I will prove that all other write usage is not impacted later. Thanks for all reply. Sean -- Steve 4 Support writting 1...\0 to proc file. Eric -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Tue, 25 Aug 2015 15:50:18 +0800 Sean Fu fxinr...@gmail.com wrote: On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 6:57:57 PM MDT, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. The thing is, the above argument is against the patch. The behavior changed 2 years ago, and nobody noticed. Changing it back only causes more inconsistent behavior. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. Writing to a proc file causes kernel actions. Writing to a regular file just saves data. That's not an argument here. 3 This patch does not have any potential compatibility risk with 3rd party application. How do you know that? -- Steve 4 Support writting 1...\0 to proc file. Eric -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 6:57:57 PM MDT, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. We should consider this patch basing on my following arguments. 1 Different version kernel should keep consistent on this behavior. 2 This writting behavior on proc file should be same with writting on regular file as possible as we can. 3 This patch does not have any potential compatibility risk with 3rd party application. 4 Support writting 1...\0 to proc file. Eric -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Tue, 25 Aug 2015 13:33:57 -0400 Austin S Hemmelgarn ahferro...@gmail.com wrote: How do you know that? I will prove that all other write usage is not impacted later. Except that you can only really do this for programs that you have access to, and by definition you can not have access to every program ever written that writes to /proc. If you were going to do this, it would need to be itself controlled by another sysctl to toggle the behavior, which would need to default to the current behavior. Defending the patch, I can't imagine any user space code expecting the current behavior. The current behavior is that if you write 1\0 it will error out instead of accepting the 1. I can't come up with a scenario that would require userspace to expect 1\0 to fail. Can you? -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On 24.08.2015 10:56, Sean Fu wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Reading through kernel/sysctl.c it looks like you are allowing 1\01 to be used to pass two integers or two longs. This is not what you describe as target of your patch. Parameter tr returned from proc_get_long should be checked in __do_proc_dointvec, __do_proc_doulongvec_minmax. Best regards Heinrich Schuchardt Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
Call path is "proc_dointvec-->do_proc_dointvec-->__do_proc_dointvec-->proc_get_long". file: kernel/sysctl.c function: proc_get_long ... 1927 if (len < *size && perm_tr_len && !memchr(perm_tr, *p, perm_tr_len)) //this line should accept two bytes "1\0". 1928 return -EINVAL; ... The latest upstream kernel is also tested, and it is same as 3.16.7 kernel. 3.16.7 kernel: sean@linux-dunz:~/work/suse_lab/proc_test> cat /proc/version Linux version 3.16.7-7-desktop (geeko@buildhost) (gcc version 4.8.3 20140627 [gcc-4_8-branch revision 212064] (SUSE Linux) ) #1 SMP PREEMPT Wed Dec 17 18:00:44 UTC 2014 (762f27a) sean@linux-dunz:~/work/suse_lab/proc_test> gcc ./proc_test.c -o proc_test sean@linux-dunz:~/work/suse_lab/proc_test> sudo ./proc_test enp0s25 Input: ./proc_test, enp0s25 file is: /proc/sys/net/ipv4/conf/enp0s25/rp_filter. open /proc/sys/net/ipv4/conf/enp0s25/rp_filter ok, fd=3 write 3: len=-1, errno=22, Invalid argument 2.6.16.60 kernel: linux-8lij:~ # gcc ./proc_test.c -o ./proc_test linux-8lij:~ # cat /proc/version Linux version 2.6.16.60-0.83.2-bigsmp (geeko@buildhost) (gcc version 4.1.2 20070115 (SUSE Linux)) #1 SMP Fri Sep 2 13:49:16 UTC 2011 linux-8lij:~ # gcc ./proc_test.c -o ./proc_test linux-8lij:~ # ./proc_test eth7 Input: ./proc_test, eth7 file is: /proc/sys/net/ipv4/conf/eth7/rp_filter. open /proc/sys/net/ipv4/conf/eth7/rp_filter ok, fd=3 write 3: len=1, errno=0, Success On Tue, Aug 25, 2015 at 8:57 AM, Sean Fu wrote: > An application from HuaWei which works fine on 2.6 encounters this > issue on 3.0 or later kernel. > > Test code: > > #include > #include > #include > #include > #include > #include > #include > > #define MAXLEN (100) > > int main(int argc, char** argv) > { > int fd = 0; > int len = 0; > char pathname[MAXLEN] = {0}; > char buf[2] = {0}; > int ret = 0xF; > int value = 0xF; > > if (argc < 2) > { > printf("Input param error, less 2 param: %s, %s.\n", > argv[0], argv[1]); > return 1; > } > > printf("Input: %s, %s \n", argv[0], argv[1]); > > ret = sprintf(pathname, > "/proc/sys/net/ipv4/conf/%s/rp_filter", argv[1]); > if (ret < 0) > printf("sprintf error, errno %d, %s\n", errno, > strerror(errno)); > printf("file is: %s. \n", pathname); > > fd = open(pathname, O_RDWR, S_IRUSR); > if (fd <=0 ) > { > printf("open %s failed, errno=%d, %s.\n", pathname, > errno, strerror(errno)); > return 1; > } > printf("open %s ok, fd=%d\n", pathname, fd); > > len = write(fd, "0\0", 2); > printf("write %d: len=%d, errno=%d, %s\n", fd, len, errno, > strerror(errno)); > > close(fd); > return 0; > } > > On Tue, Aug 25, 2015 at 12:59 AM, Steven Rostedt wrote: >> On Mon, 24 Aug 2015 16:56:13 +0800 >> Sean Fu wrote: >> >>> when the input argument "count" including the terminating byte "\0", >>> The write system call return EINVAL on proc file. >>> But it return success on regular file. >>> >>> E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". >>> write(fd, "1\0", 2) return EINVAL. >> >> And what would do that? What tool broke because of this? >> >> echo 1 > /proc/sys/net/ipv4/conf/eth0/rp_filter >> >> works just fine. strlen("string") would not include the nul character. >> The only thing I could think of would be a sizeof(str), but then that >> would include someone hardcoding an integer in a string, like: >> >> char val[] = "1" >> >> write(fd, val, sizeof(val)); >> >> Again, what tool does that? >> >> If there is a tool out in the wild that use to work on 2.6 (and was >> running on 2.6 then, and not something that was created after that >> change), then we can consider this fix. >> >> -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On August 24, 2015 6:57:57 PM MDT, Sean Fu wrote: >An application from HuaWei which works fine on 2.6 encounters this >issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. Eric -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. Test code: #include #include #include #include #include #include #include #define MAXLEN (100) int main(int argc, char** argv) { int fd = 0; int len = 0; char pathname[MAXLEN] = {0}; char buf[2] = {0}; int ret = 0xF; int value = 0xF; if (argc < 2) { printf("Input param error, less 2 param: %s, %s.\n", argv[0], argv[1]); return 1; } printf("Input: %s, %s \n", argv[0], argv[1]); ret = sprintf(pathname, "/proc/sys/net/ipv4/conf/%s/rp_filter", argv[1]); if (ret < 0) printf("sprintf error, errno %d, %s\n", errno, strerror(errno)); printf("file is: %s. \n", pathname); fd = open(pathname, O_RDWR, S_IRUSR); if (fd <=0 ) { printf("open %s failed, errno=%d, %s.\n", pathname, errno, strerror(errno)); return 1; } printf("open %s ok, fd=%d\n", pathname, fd); len = write(fd, "0\0", 2); printf("write %d: len=%d, errno=%d, %s\n", fd, len, errno, strerror(errno)); close(fd); return 0; } On Tue, Aug 25, 2015 at 12:59 AM, Steven Rostedt wrote: > On Mon, 24 Aug 2015 16:56:13 +0800 > Sean Fu wrote: > >> when the input argument "count" including the terminating byte "\0", >> The write system call return EINVAL on proc file. >> But it return success on regular file. >> >> E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". >> write(fd, "1\0", 2) return EINVAL. > > And what would do that? What tool broke because of this? > > echo 1 > /proc/sys/net/ipv4/conf/eth0/rp_filter > > works just fine. strlen("string") would not include the nul character. > The only thing I could think of would be a sizeof(str), but then that > would include someone hardcoding an integer in a string, like: > > char val[] = "1" > > write(fd, val, sizeof(val)); > > Again, what tool does that? > > If there is a tool out in the wild that use to work on 2.6 (and was > running on 2.6 then, and not something that was created after that > change), then we can consider this fix. > > -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
This seems to be the relevant patch: https://lkml.org/lkml/2010/5/5/104 Amerigo Wang 2010-05-05 02:26:45 00b7c3395aec3df43de5bd02a3c5a099ca51169f +static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; So since 2010 we have the current behavior. Best regards Heinrich Schuchardt On 24.08.2015 22:44, Andrew Morton wrote: > On Mon, 24 Aug 2015 23:33:58 +0800 Sean Fu wrote: > >> On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman >> wrote: >>> >>> >>> On August 24, 2015 1:56:13 AM PDT, Sean Fu wrote: when the input argument "count" including the terminating byte "\0", The write system call return EINVAL on proc file. But it return success on regular file. >>> >>> Nonsense. It will write the '\0' to a regular file because it is just data. >>> >>> Integers in proc are more than data. >>> >>> So I see no justification for this change. >> In fact, "write(fd, "1\0", 2)" on Integers proc file return success on >> 2.6 kernel. I already tested it on 2.6.6.60 kernel. >> >> So, The latest behavior of "write(fd, "1\0", 2)" is different from old >> kernel(2.6). >> This maybe impact the compatibility of some user space program. > > 2.6 was a long time ago. If this behaviour change has happened in the > last 1-2 kernel releases then there would be a case to consider making > changes. But if the kernel has been this way for two years then it's > too late to bother switching back to the old (and strange) behaviour. > > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Mon, 24 Aug 2015 23:33:58 +0800 Sean Fu wrote: > On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman > wrote: > > > > > > On August 24, 2015 1:56:13 AM PDT, Sean Fu wrote: > >>when the input argument "count" including the terminating byte "\0", > >>The write system call return EINVAL on proc file. > >>But it return success on regular file. > > > > Nonsense. It will write the '\0' to a regular file because it is just data. > > > > Integers in proc are more than data. > > > > So I see no justification for this change. > In fact, "write(fd, "1\0", 2)" on Integers proc file return success on > 2.6 kernel. I already tested it on 2.6.6.60 kernel. > > So, The latest behavior of "write(fd, "1\0", 2)" is different from old > kernel(2.6). > This maybe impact the compatibility of some user space program. 2.6 was a long time ago. If this behaviour change has happened in the last 1-2 kernel releases then there would be a case to consider making changes. But if the kernel has been this way for two years then it's too late to bother switching back to the old (and strange) behaviour. -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Mon, 24 Aug 2015 16:56:13 +0800 Sean Fu wrote: > when the input argument "count" including the terminating byte "\0", > The write system call return EINVAL on proc file. > But it return success on regular file. > > E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". > write(fd, "1\0", 2) return EINVAL. And what would do that? What tool broke because of this? echo 1 > /proc/sys/net/ipv4/conf/eth0/rp_filter works just fine. strlen("string") would not include the nul character. The only thing I could think of would be a sizeof(str), but then that would include someone hardcoding an integer in a string, like: char val[] = "1" write(fd, val, sizeof(val)); Again, what tool does that? If there is a tool out in the wild that use to work on 2.6 (and was running on 2.6 then, and not something that was created after that change), then we can consider this fix. -- Steve -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman wrote: > > > On August 24, 2015 1:56:13 AM PDT, Sean Fu wrote: >>when the input argument "count" including the terminating byte "\0", >>The write system call return EINVAL on proc file. >>But it return success on regular file. > > Nonsense. It will write the '\0' to a regular file because it is just data. > > Integers in proc are more than data. > > So I see no justification for this change. In fact, "write(fd, "1\0", 2)" on Integers proc file return success on 2.6 kernel. I already tested it on 2.6.6.60 kernel. So, The latest behavior of "write(fd, "1\0", 2)" is different from old kernel(2.6). This maybe impact the compatibility of some user space program. > > > Eric > >>E.g. Writting two bytes ("1\0") to >>"/proc/sys/net/ipv4/conf/eth0/rp_filter". >>write(fd, "1\0", 2) return EINVAL. >> >>Signed-off-by: Sean Fu >>--- >> kernel/sysctl.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >>diff --git a/kernel/sysctl.c b/kernel/sysctl.c >>index 19b62b5..c2b0594 100644 >>--- a/kernel/sysctl.c >>+++ b/kernel/sysctl.c >>@@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, >>unsigned long *lvalp, >>return 0; >> } >> >>-static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; >>+static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; >> >> static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, >> int write, void __user *buffer, > -- 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: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
On August 24, 2015 1:56:13 AM PDT, Sean Fu wrote: >when the input argument "count" including the terminating byte "\0", >The write system call return EINVAL on proc file. >But it return success on regular file. Nonsense. It will write the '\0' to a regular file because it is just data. Integers in proc are more than data. So I see no justification for this change. Eric >E.g. Writting two bytes ("1\0") to >"/proc/sys/net/ipv4/conf/eth0/rp_filter". >write(fd, "1\0", 2) return EINVAL. > >Signed-off-by: Sean Fu >--- > kernel/sysctl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/kernel/sysctl.c b/kernel/sysctl.c >index 19b62b5..c2b0594 100644 >--- a/kernel/sysctl.c >+++ b/kernel/sysctl.c >@@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, >unsigned long *lvalp, >return 0; > } > >-static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; >+static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; > > static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, > int write, void __user *buffer, -- 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/
[PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.
when the input argument "count" including the terminating byte "\0", The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes ("1\0") to "/proc/sys/net/ipv4/conf/eth0/rp_filter". write(fd, "1\0", 2) return EINVAL. Signed-off-by: Sean Fu --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 2.1.2 -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On August 24, 2015 1:56:13 AM PDT, Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. Nonsense. It will write the '\0' to a regular file because it is just data. Integers in proc are more than data. So I see no justification for this change. Eric E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 1:56:13 AM PDT, Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. Nonsense. It will write the '\0' to a regular file because it is just data. Integers in proc are more than data. So I see no justification for this change. In fact, write(fd, 1\0, 2) on Integers proc file return success on 2.6 kernel. I already tested it on 2.6.6.60 kernel. So, The latest behavior of write(fd, 1\0, 2) is different from old kernel(2.6). This maybe impact the compatibility of some user space program. Eric E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Mon, 24 Aug 2015 16:56:13 +0800 Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. And what would do that? What tool broke because of this? echo 1 /proc/sys/net/ipv4/conf/eth0/rp_filter works just fine. strlen(string) would not include the nul character. The only thing I could think of would be a sizeof(str), but then that would include someone hardcoding an integer in a string, like: char val[] = 1 write(fd, val, sizeof(val)); Again, what tool does that? If there is a tool out in the wild that use to work on 2.6 (and was running on 2.6 then, and not something that was created after that change), then we can consider this fix. -- Steve -- 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/
[PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. Signed-off-by: Sean Fu fxinr...@gmail.com --- kernel/sysctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 19b62b5..c2b0594 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -2004,7 +2004,7 @@ static int do_proc_dointvec_conv(bool *negp, unsigned long *lvalp, return 0; } -static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; +static const char proc_wspace_sep[] = { ' ', '\t', '\n', '\0' }; static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, int write, void __user *buffer, -- 2.1.2 -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On Mon, 24 Aug 2015 23:33:58 +0800 Sean Fu fxinr...@gmail.com wrote: On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 1:56:13 AM PDT, Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. Nonsense. It will write the '\0' to a regular file because it is just data. Integers in proc are more than data. So I see no justification for this change. In fact, write(fd, 1\0, 2) on Integers proc file return success on 2.6 kernel. I already tested it on 2.6.6.60 kernel. So, The latest behavior of write(fd, 1\0, 2) is different from old kernel(2.6). This maybe impact the compatibility of some user space program. 2.6 was a long time ago. If this behaviour change has happened in the last 1-2 kernel releases then there would be a case to consider making changes. But if the kernel has been this way for two years then it's too late to bother switching back to the old (and strange) behaviour. -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
This seems to be the relevant patch: https://lkml.org/lkml/2010/5/5/104 Amerigo Wang amw...@redhat.com 2010-05-05 02:26:45 00b7c3395aec3df43de5bd02a3c5a099ca51169f +static const char proc_wspace_sep[] = { ' ', '\t', '\n' }; So since 2010 we have the current behavior. Best regards Heinrich Schuchardt On 24.08.2015 22:44, Andrew Morton wrote: On Mon, 24 Aug 2015 23:33:58 +0800 Sean Fu fxinr...@gmail.com wrote: On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman ebied...@xmission.com wrote: On August 24, 2015 1:56:13 AM PDT, Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. Nonsense. It will write the '\0' to a regular file because it is just data. Integers in proc are more than data. So I see no justification for this change. In fact, write(fd, 1\0, 2) on Integers proc file return success on 2.6 kernel. I already tested it on 2.6.6.60 kernel. So, The latest behavior of write(fd, 1\0, 2) is different from old kernel(2.6). This maybe impact the compatibility of some user space program. 2.6 was a long time ago. If this behaviour change has happened in the last 1-2 kernel releases then there would be a case to consider making changes. But if the kernel has been this way for two years then it's too late to bother switching back to the old (and strange) behaviour. -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. Test code: #include sys/types.h #include sys/stat.h #include fcntl.h #include unistd.h #include stdio.h #include string.h #include errno.h #define MAXLEN (100) int main(int argc, char** argv) { int fd = 0; int len = 0; char pathname[MAXLEN] = {0}; char buf[2] = {0}; int ret = 0xF; int value = 0xF; if (argc 2) { printf(Input param error, less 2 param: %s, %s.\n, argv[0], argv[1]); return 1; } printf(Input: %s, %s \n, argv[0], argv[1]); ret = sprintf(pathname, /proc/sys/net/ipv4/conf/%s/rp_filter, argv[1]); if (ret 0) printf(sprintf error, errno %d, %s\n, errno, strerror(errno)); printf(file is: %s. \n, pathname); fd = open(pathname, O_RDWR, S_IRUSR); if (fd =0 ) { printf(open %s failed, errno=%d, %s.\n, pathname, errno, strerror(errno)); return 1; } printf(open %s ok, fd=%d\n, pathname, fd); len = write(fd, 0\0, 2); printf(write %d: len=%d, errno=%d, %s\n, fd, len, errno, strerror(errno)); close(fd); return 0; } On Tue, Aug 25, 2015 at 12:59 AM, Steven Rostedt rost...@goodmis.org wrote: On Mon, 24 Aug 2015 16:56:13 +0800 Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. And what would do that? What tool broke because of this? echo 1 /proc/sys/net/ipv4/conf/eth0/rp_filter works just fine. strlen(string) would not include the nul character. The only thing I could think of would be a sizeof(str), but then that would include someone hardcoding an integer in a string, like: char val[] = 1 write(fd, val, sizeof(val)); Again, what tool does that? If there is a tool out in the wild that use to work on 2.6 (and was running on 2.6 then, and not something that was created after that change), then we can consider this fix. -- Steve -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
On August 24, 2015 6:57:57 PM MDT, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. My sympathies. Being stuck with a 3rd party application you can barely talk about that has been broken for 5years and no one reported it. Ordinarily we would fix a regression like this. As it has been 5years the challenge now is how do we tell if there are applications that depend on the current behavior. Before we can change the behavior back we need a convincing argument that we won't cause a regression in another application by making the change. I do not see how such an argument can be made. So you have my sympathies but I do not see how we can help you. Eric -- 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: [PATCH] kernel/sysctl.c: If count including the terminating byte '\0' the write system call should retrun success.
Call path is proc_dointvec--do_proc_dointvec--__do_proc_dointvec--proc_get_long. file: kernel/sysctl.c function: proc_get_long ... 1927 if (len *size perm_tr_len !memchr(perm_tr, *p, perm_tr_len)) //this line should accept two bytes 1\0. 1928 return -EINVAL; ... The latest upstream kernel is also tested, and it is same as 3.16.7 kernel. 3.16.7 kernel: sean@linux-dunz:~/work/suse_lab/proc_test cat /proc/version Linux version 3.16.7-7-desktop (geeko@buildhost) (gcc version 4.8.3 20140627 [gcc-4_8-branch revision 212064] (SUSE Linux) ) #1 SMP PREEMPT Wed Dec 17 18:00:44 UTC 2014 (762f27a) sean@linux-dunz:~/work/suse_lab/proc_test gcc ./proc_test.c -o proc_test sean@linux-dunz:~/work/suse_lab/proc_test sudo ./proc_test enp0s25 Input: ./proc_test, enp0s25 file is: /proc/sys/net/ipv4/conf/enp0s25/rp_filter. open /proc/sys/net/ipv4/conf/enp0s25/rp_filter ok, fd=3 write 3: len=-1, errno=22, Invalid argument 2.6.16.60 kernel: linux-8lij:~ # gcc ./proc_test.c -o ./proc_test linux-8lij:~ # cat /proc/version Linux version 2.6.16.60-0.83.2-bigsmp (geeko@buildhost) (gcc version 4.1.2 20070115 (SUSE Linux)) #1 SMP Fri Sep 2 13:49:16 UTC 2011 linux-8lij:~ # gcc ./proc_test.c -o ./proc_test linux-8lij:~ # ./proc_test eth7 Input: ./proc_test, eth7 file is: /proc/sys/net/ipv4/conf/eth7/rp_filter. open /proc/sys/net/ipv4/conf/eth7/rp_filter ok, fd=3 write 3: len=1, errno=0, Success On Tue, Aug 25, 2015 at 8:57 AM, Sean Fu fxinr...@gmail.com wrote: An application from HuaWei which works fine on 2.6 encounters this issue on 3.0 or later kernel. Test code: #include sys/types.h #include sys/stat.h #include fcntl.h #include unistd.h #include stdio.h #include string.h #include errno.h #define MAXLEN (100) int main(int argc, char** argv) { int fd = 0; int len = 0; char pathname[MAXLEN] = {0}; char buf[2] = {0}; int ret = 0xF; int value = 0xF; if (argc 2) { printf(Input param error, less 2 param: %s, %s.\n, argv[0], argv[1]); return 1; } printf(Input: %s, %s \n, argv[0], argv[1]); ret = sprintf(pathname, /proc/sys/net/ipv4/conf/%s/rp_filter, argv[1]); if (ret 0) printf(sprintf error, errno %d, %s\n, errno, strerror(errno)); printf(file is: %s. \n, pathname); fd = open(pathname, O_RDWR, S_IRUSR); if (fd =0 ) { printf(open %s failed, errno=%d, %s.\n, pathname, errno, strerror(errno)); return 1; } printf(open %s ok, fd=%d\n, pathname, fd); len = write(fd, 0\0, 2); printf(write %d: len=%d, errno=%d, %s\n, fd, len, errno, strerror(errno)); close(fd); return 0; } On Tue, Aug 25, 2015 at 12:59 AM, Steven Rostedt rost...@goodmis.org wrote: On Mon, 24 Aug 2015 16:56:13 +0800 Sean Fu fxinr...@gmail.com wrote: when the input argument count including the terminating byte \0, The write system call return EINVAL on proc file. But it return success on regular file. E.g. Writting two bytes (1\0) to /proc/sys/net/ipv4/conf/eth0/rp_filter. write(fd, 1\0, 2) return EINVAL. And what would do that? What tool broke because of this? echo 1 /proc/sys/net/ipv4/conf/eth0/rp_filter works just fine. strlen(string) would not include the nul character. The only thing I could think of would be a sizeof(str), but then that would include someone hardcoding an integer in a string, like: char val[] = 1 write(fd, val, sizeof(val)); Again, what tool does that? If there is a tool out in the wild that use to work on 2.6 (and was running on 2.6 then, and not something that was created after that change), then we can consider this fix. -- Steve -- 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/