Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1211929 > unrolled thread

[PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.

Back to article view | Back to linux.kernel

Contents

  [PATCH] kernel/sysctl.c: If "count" including the terminating byte  '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-24 11:00 +0200
    Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success. "Eric W. Biederman" <ebiederm@xmission.com> - 2015-08-24 14:30 +0200
      Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-24 17:40 +0200
        Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Andrew Morton <akpm@linux-foundation.org> - 2015-08-24 22:50 +0200
          Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Heinrich Schuchardt <xypron.glpk@gmx.de> - 2015-08-24 23:30 +0200
    Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Steven Rostedt <rostedt@goodmis.org> - 2015-08-24 19:00 +0200
      Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-25 03:00 +0200
        Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success. "Eric W. Biederman" <ebiederm@xmission.com> - 2015-08-25 04:30 +0200
          Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-25 10:10 +0200
            Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Steven Rostedt <rostedt@goodmis.org> - 2015-08-25 16:20 +0200
              Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-25 18:50 +0200
                Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Austin S Hemmelgarn <ahferroin7@gmail.com> - 2015-08-25 19:40 +0200
                  Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Steven Rostedt <rostedt@goodmis.org> - 2015-08-25 21:10 +0200
                    Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-26 17:50 +0200
                      Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Steven Rostedt <rostedt@goodmis.org> - 2015-08-26 22:40 +0200
                        Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-27 02:20 +0200
                          Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Steven Rostedt <rostedt@goodmis.org> - 2015-08-27 04:40 +0200
                            Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-27 10:40 +0200
                              Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-28 05:40 +0200
        Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-25 05:20 +0200
    Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Heinrich Schuchardt <xypron.glpk@gmx.de> - 2015-08-25 22:50 +0200
      Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-26 11:40 +0200
      Re: [PATCH] kernel/sysctl.c: If "count" including the terminating  byte '\0' the write system call should retrun success. Sean Fu <fxinrong@gmail.com> - 2015-08-27 02:40 +0200

Page 1 of 2  [1] 2  Next page →

#1211929 — [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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1212115 — 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 <fxinrong@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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212317 — 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
<ebiederm@xmission.com> wrote:
>
>
> On August 24, 2015 1:56:13 AM PDT, Sean Fu <fxinrong@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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212494 — 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 <fxinrong@gmail.com> wrote:

> On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
> >
> >
> > On August 24, 2015 1:56:13 AM PDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212520 — 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 <amwang@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 <fxinrong@gmail.com> wrote:
> 
>> On Mon, Aug 24, 2015 at 8:27 PM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>>>
>>>
>>> On August 24, 2015 1:56:13 AM PDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212345 — 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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212616 — 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 <rostedt@goodmis.org> wrote:
> On Mon, 24 Aug 2015 16:56:13 +0800
> Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212649 — 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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212782 — 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
<ebiederm@xmission.com> wrote:
>
>
> On August 24, 2015 6:57:57 PM MDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213067 — 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 <fxinrong@gmail.com> wrote:

> On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman
> <ebiederm@xmission.com> wrote:
> >
> >
> > On August 24, 2015 6:57:57 PM MDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213181 — 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 <rostedt@goodmis.org> wrote:
> On Tue, 25 Aug 2015 15:50:18 +0800
> Sean Fu <fxinrong@gmail.com> wrote:
>
>> On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman
>> <ebiederm@xmission.com> wrote:
>> >
>> >
>> > On August 24, 2015 6:57:57 PM MDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213205 — Re: [PATCH] kernel/sysctl.c: If "count" including the terminating byte '\0' the write system call should retrun success.

[Multipart message — attachments visible in raw view] — view raw

On 2015-08-25 12:44, Sean Fu wrote:
> On Tue, Aug 25, 2015 at 10:15 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Tue, 25 Aug 2015 15:50:18 +0800
>> Sean Fu <fxinrong@gmail.com> wrote:
>>
>>> On Tue, Aug 25, 2015 at 10:24 AM, Eric W. Biederman
>>> <ebiederm@xmission.com> wrote:
>>>>
>>>>
>>>> On August 24, 2015 6:57:57 PM MDT, Sean Fu <fxinrong@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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

[toc] | [prev] | [next] | [standalone]

#1213258 — 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 <ahferroin7@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1213972 — 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 <rostedt@goodmis.org> wrote:
> On Tue, 25 Aug 2015 13:33:57 -0400
> Austin S Hemmelgarn <ahferroin7@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214178 — 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 <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214255 — 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 <rostedt@goodmis.org> wrote:
> On Wed, 26 Aug 2015 23:48:01 +0800
> Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214295 — 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 <fxinrong@gmail.com> wrote:

> On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Wed, 26 Aug 2015 23:48:01 +0800
> > Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214383 — 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 <rostedt@goodmis.org> wrote:
> On Thu, 27 Aug 2015 08:17:29 +0800
> Sean Fu <fxinrong@gmail.com> wrote:
>
>> On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > On Wed, 26 Aug 2015 23:48:01 +0800
>> > Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1215077 — 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 <fxinrong@gmail.com> wrote:
> On Thu, Aug 27, 2015 at 10:32 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Thu, 27 Aug 2015 08:17:29 +0800
>> Sean Fu <fxinrong@gmail.com> wrote:
>>
>>> On Thu, Aug 27, 2015 at 4:36 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>>> > On Wed, 26 Aug 2015 23:48:01 +0800
>>> > Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1212681 — 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 <fxinrong@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 <rostedt@goodmis.org> wrote:
>> On Mon, 24 Aug 2015 16:56:13 +0800
>> Sean Fu <fxinrong@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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web