* [PATCH v2] sctp: check the rto_min and rto_max @ 2013-12-02 1:45 Wang Weidong 2013-12-02 12:15 ` Wang Weidong 2013-12-02 14:45 ` Vlad Yasevich 0 siblings, 2 replies; 8+ messages in thread From: Wang Weidong @ 2013-12-02 1:45 UTC (permalink / raw) To: Vlad Yasevich, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong rto_min should be smaller than rto_max while rto_max should be larger than rto_min. so just add the check. Acked-by: Neil Horman <nhorman@tuxdriver.com> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> --- net/sctp/sysctl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c index 6b36561..7637e8e 100644 --- a/net/sctp/sysctl.c +++ b/net/sctp/sysctl.c @@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &one, - .extra2 = &timer_max + .extra2 = &init_net.sctp.rto_max }, { .procname = "rto_max", @@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &one, + .extra1 = &init_net.sctp.rto_min, .extra2 = &timer_max }, { -- 1.7.12 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sctp: check the rto_min and rto_max 2013-12-02 1:45 [PATCH v2] sctp: check the rto_min and rto_max Wang Weidong @ 2013-12-02 12:15 ` Wang Weidong 2013-12-02 14:45 ` Vlad Yasevich 1 sibling, 0 replies; 8+ messages in thread From: Wang Weidong @ 2013-12-02 12:15 UTC (permalink / raw) To: Vlad Yasevich, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong On 2013/12/2 9:45, Wang Weidong wrote: > rto_min should be smaller than rto_max while rto_max should be larger > than rto_min. so just add the check. > > Acked-by: Neil Horman <nhorman@tuxdriver.com> > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/sctp/sysctl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c > index 6b36561..7637e8e 100644 > --- a/net/sctp/sysctl.c > +++ b/net/sctp/sysctl.c > @@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = { > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > .extra1 = &one, > - .extra2 = &timer_max > + .extra2 = &init_net.sctp.rto_max > }, > { > .procname = "rto_max", > @@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > + .extra1 = &init_net.sctp.rto_min, > .extra2 = &timer_max > }, > { > Hi Neil, As you point out that in "[PATCH] sctp: make the max_burst min value to 1": > 1) You can also set the the max_burst via setsockopt, and so this would need to > be checked in that path as well. we can set the rto_min/rto_max via setsockopt. Should I check it in the path as well? Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sctp: check the rto_min and rto_max 2013-12-02 1:45 [PATCH v2] sctp: check the rto_min and rto_max Wang Weidong 2013-12-02 12:15 ` Wang Weidong @ 2013-12-02 14:45 ` Vlad Yasevich 2013-12-03 1:58 ` Wang Weidong 1 sibling, 1 reply; 8+ messages in thread From: Vlad Yasevich @ 2013-12-02 14:45 UTC (permalink / raw) To: Wang Weidong, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong On 12/01/2013 08:45 PM, Wang Weidong wrote: > rto_min should be smaller than rto_max while rto_max should be larger > than rto_min. so just add the check. > > Acked-by: Neil Horman <nhorman@tuxdriver.com> > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/sctp/sysctl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c > index 6b36561..7637e8e 100644 > --- a/net/sctp/sysctl.c > +++ b/net/sctp/sysctl.c > @@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = { > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > .extra1 = &one, > - .extra2 = &timer_max > + .extra2 = &init_net.sctp.rto_max > }, > { > .procname = "rto_max", > @@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > + .extra1 = &init_net.sctp.rto_min, > .extra2 = &timer_max > }, > { > So, now if initial namespace change rto_min and rto_max, it limits the values that another namespace can set? I don't think so. If you are looking to fix an issue where rto_max may be set below rto_min, then do a separate proc_handler. -vlad ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] sctp: check the rto_min and rto_max 2013-12-02 14:45 ` Vlad Yasevich @ 2013-12-03 1:58 ` Wang Weidong 2013-12-03 6:28 ` [PATCH v3] " Wang Weidong 0 siblings, 1 reply; 8+ messages in thread From: Wang Weidong @ 2013-12-03 1:58 UTC (permalink / raw) To: Vlad Yasevich, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong On 2013/12/2 22:45, Vlad Yasevich wrote: > On 12/01/2013 08:45 PM, Wang Weidong wrote: >> rto_min should be smaller than rto_max while rto_max should be larger >> than rto_min. so just add the check. >> >> Acked-by: Neil Horman <nhorman@tuxdriver.com> >> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >> --- >> net/sctp/sysctl.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c >> index 6b36561..7637e8e 100644 >> --- a/net/sctp/sysctl.c >> +++ b/net/sctp/sysctl.c >> @@ -104,7 +104,7 @@ static struct ctl_table sctp_net_table[] = { >> .mode = 0644, >> .proc_handler = proc_dointvec_minmax, >> .extra1 = &one, >> - .extra2 = &timer_max >> + .extra2 = &init_net.sctp.rto_max >> }, >> { >> .procname = "rto_max", >> @@ -112,7 +112,7 @@ static struct ctl_table sctp_net_table[] = { >> .maxlen = sizeof(unsigned int), >> .mode = 0644, >> .proc_handler = proc_dointvec_minmax, >> - .extra1 = &one, >> + .extra1 = &init_net.sctp.rto_min, >> .extra2 = &timer_max >> }, >> { >> > > So, now if initial namespace change rto_min and rto_max, it limits the > values that another namespace can set? I don't think so. Hm, you are right. I think we have two path to set the rto_min or rto_max: 1)systclt, 2)setsockopt In 2) we can make the assoc or sk rto_max smaller than rto_min. so we should a check in the sctp_setsockopt_rtoinfo or proc_handler. > > If you are looking to fix an issue where rto_max may be set below > rto_min, then do a separate proc_handler. I will try to add a separate proc_handler. Thanks. > > -vlad > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] sctp: check the rto_min and rto_max 2013-12-03 1:58 ` Wang Weidong @ 2013-12-03 6:28 ` Wang Weidong 2013-12-03 12:39 ` Vlad Yasevich 0 siblings, 1 reply; 8+ messages in thread From: Wang Weidong @ 2013-12-03 6:28 UTC (permalink / raw) To: Vlad Yasevich, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong rto_min should be smaller than rto_max while rto_max should be larger than rto_min. Add two proc_handler for the checking. Add the check in sctp_setsockopt_rtoinfo. Suggested-by: Vlad Yasevich <vyasevich@gmail.com> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> --- net/sctp/socket.c | 5 +++++ net/sctp/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 72046b9..2e1af1b 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -2818,6 +2818,11 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne if (copy_from_user(&rtoinfo, optval, optlen)) return -EFAULT; + if (rtoinfo.srto_min < 1 || + rtoinfo.srto_max > 86400000 || + rtoinfo.srto_max < rtoinfo.srto_min) + return -EINVAL; + asoc = sctp_id2assoc(sk, rtoinfo.srto_assoc_id); /* Set the values to the specific association */ diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c index 6b36561..525b06d 100644 --- a/net/sctp/sysctl.c +++ b/net/sctp/sysctl.c @@ -59,8 +59,16 @@ extern int sysctl_sctp_wmem[3]; static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, int write, void __user *buffer, size_t *lenp, - loff_t *ppos); +static int proc_sctp_check_rtomin(struct ctl_table *ctl, + int write, + void __user *buffer, size_t *lenp, + loff_t *ppos); +static int proc_sctp_check_rtomax(struct ctl_table *ctl, + int write, + void __user *buffer, size_t *lenp, + loff_t *ppos); + static struct ctl_table sctp_table[] = { { .procname = "sctp_mem", @@ -102,18 +110,14 @@ static struct ctl_table sctp_net_table[] = { .data = &init_net.sctp.rto_min, .maxlen = sizeof(unsigned int), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &one, - .extra2 = &timer_max + .proc_handler = proc_sctp_check_rtomin, }, { .procname = "rto_max", .data = &init_net.sctp.rto_max, .maxlen = sizeof(unsigned int), .mode = 0644, - .proc_handler = proc_dointvec_minmax, - .extra1 = &one, - .extra2 = &timer_max + .proc_handler = proc_sctp_check_rtomax, }, { .procname = "rto_alpha_exp_divisor", @@ -342,6 +346,46 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, return ret; } +static int proc_sctp_check_rtomin(struct ctl_table *ctl, + int write, + void __user *buffer, size_t *lenp, + loff_t *ppos) +{ + int old_value = *(int *) ctl->data; + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); + + if (write) { + int new_value = *(int *) ctl->data; + if (ret || new_value < one || new_value > init_net.sctp.rto_max) { + init_net.sctp.rto_min = old_value; + return -EINVAL; + } + init_net.sctp.rto_min = new_value; + } + + return ret; +} + +static int proc_sctp_check_rtomax(struct ctl_table *ctl, + int write, + void __user*buffer, size_t *lenp, + loff_t *ppos) +{ + int old_value = *(int *) ctl->data; + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); + + if (write) { + int new_value = *(int *) ctl->data; + if (ret || new_value > timer_max || new_value < init_net.sctp.rto_min) { + init_net.sctp.rto_max = old_value; + return -EINVAL; + } + init_net.sctp.rto_max = new_value; + } + + return ret; +} + int sctp_sysctl_net_register(struct net *net) { struct ctl_table *table; -- 1.7.12 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] sctp: check the rto_min and rto_max 2013-12-03 6:28 ` [PATCH v3] " Wang Weidong @ 2013-12-03 12:39 ` Vlad Yasevich 2013-12-04 2:46 ` Wang Weidong 0 siblings, 1 reply; 8+ messages in thread From: Vlad Yasevich @ 2013-12-03 12:39 UTC (permalink / raw) To: Wang Weidong, nhorman, David Miller; +Cc: linux-sctp, netdev, dingtianhong On 12/03/2013 01:28 AM, Wang Weidong wrote: > rto_min should be smaller than rto_max while rto_max should be larger > than rto_min. Add two proc_handler for the checking. Add the check in > sctp_setsockopt_rtoinfo. > > Suggested-by: Vlad Yasevich <vyasevich@gmail.com> > Signed-off-by: Wang Weidong <wangweidong1@huawei.com> > --- > net/sctp/socket.c | 5 +++++ > net/sctp/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++------- > 2 files changed, 56 insertions(+), 7 deletions(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 72046b9..2e1af1b 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -2818,6 +2818,11 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne > if (copy_from_user(&rtoinfo, optval, optlen)) > return -EFAULT; > > + if (rtoinfo.srto_min < 1 || > + rtoinfo.srto_max > 86400000 || > + rtoinfo.srto_max < rtoinfo.srto_min) > + return -EINVAL; > + > asoc = sctp_id2assoc(sk, rtoinfo.srto_assoc_id); > > /* Set the values to the specific association */ > diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c > index 6b36561..525b06d 100644 > --- a/net/sctp/sysctl.c > +++ b/net/sctp/sysctl.c > @@ -59,8 +59,16 @@ extern int sysctl_sctp_wmem[3]; > static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, > int write, > void __user *buffer, size_t *lenp, > - > loff_t *ppos); > +static int proc_sctp_check_rtomin(struct ctl_table *ctl, > + int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos); > +static int proc_sctp_check_rtomax(struct ctl_table *ctl, > + int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos); > + > static struct ctl_table sctp_table[] = { > { > .procname = "sctp_mem", > @@ -102,18 +110,14 @@ static struct ctl_table sctp_net_table[] = { > .data = &init_net.sctp.rto_min, > .maxlen = sizeof(unsigned int), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > - .extra2 = &timer_max > + .proc_handler = proc_sctp_check_rtomin, > }, > { > .procname = "rto_max", > .data = &init_net.sctp.rto_max, > .maxlen = sizeof(unsigned int), > .mode = 0644, > - .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > - .extra2 = &timer_max > + .proc_handler = proc_sctp_check_rtomax, > }, > { > .procname = "rto_alpha_exp_divisor", > @@ -342,6 +346,46 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, > return ret; > } > > +static int proc_sctp_check_rtomin(struct ctl_table *ctl, > + int write, > + void __user *buffer, size_t *lenp, > + loff_t *ppos) > +{ > + int old_value = *(int *) ctl->data; > + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); > + > + if (write) { > + int new_value = *(int *) ctl->data; > + if (ret || new_value < one || new_value > init_net.sctp.rto_max) { > + init_net.sctp.rto_min = old_value; > + return -EINVAL; > + } > + init_net.sctp.rto_min = new_value; > + } > + > + return ret; > +} > + > +static int proc_sctp_check_rtomax(struct ctl_table *ctl, > + int write, > + void __user*buffer, size_t *lenp, > + loff_t *ppos) > +{ > + int old_value = *(int *) ctl->data; > + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); > + > + if (write) { > + int new_value = *(int *) ctl->data; > + if (ret || new_value > timer_max || new_value < init_net.sctp.rto_min) { > + init_net.sctp.rto_max = old_value; > + return -EINVAL; > + } > + init_net.sctp.rto_max = new_value; > + } > + > + return ret; > +} > + You can't used init_net directly. You have to use .data as the target of assignment and .extra1 and .extra2 as the min and max boundries. Otherwise, this has the same problem as before where assignments for created namespace try to change the values in the initial namespace. -vlad > int sctp_sysctl_net_register(struct net *net) > { > struct ctl_table *table; > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] sctp: check the rto_min and rto_max 2013-12-03 12:39 ` Vlad Yasevich @ 2013-12-04 2:46 ` Wang Weidong 2013-12-04 14:24 ` Vlad Yasevich 0 siblings, 1 reply; 8+ messages in thread From: Wang Weidong @ 2013-12-04 2:46 UTC (permalink / raw) To: Vlad Yasevich, nhorman, David Miller; +Cc: linux-sctp, netdev On 2013/12/3 20:39, Vlad Yasevich wrote: > On 12/03/2013 01:28 AM, Wang Weidong wrote: >> rto_min should be smaller than rto_max while rto_max should be larger >> than rto_min. Add two proc_handler for the checking. Add the check in >> sctp_setsockopt_rtoinfo. >> >> Suggested-by: Vlad Yasevich <vyasevich@gmail.com> >> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >> --- >> net/sctp/socket.c | 5 +++++ >> net/sctp/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++------- >> 2 files changed, 56 insertions(+), 7 deletions(-) >> >> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >> index 72046b9..2e1af1b 100644 >> --- a/net/sctp/socket.c >> +++ b/net/sctp/socket.c >> @@ -2818,6 +2818,11 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne >> if (copy_from_user(&rtoinfo, optval, optlen)) >> return -EFAULT; >> >> + if (rtoinfo.srto_min < 1 || >> + rtoinfo.srto_max > 86400000 || >> + rtoinfo.srto_max < rtoinfo.srto_min) >> + return -EINVAL; >> + >> asoc = sctp_id2assoc(sk, rtoinfo.srto_assoc_id); >> >> /* Set the values to the specific association */ >> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c >> index 6b36561..525b06d 100644 >> --- a/net/sctp/sysctl.c >> +++ b/net/sctp/sysctl.c >> @@ -59,8 +59,16 @@ extern int sysctl_sctp_wmem[3]; >> static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, >> int write, >> void __user *buffer, size_t *lenp, >> - >> loff_t *ppos); >> +static int proc_sctp_check_rtomin(struct ctl_table *ctl, >> + int write, >> + void __user *buffer, size_t *lenp, >> + loff_t *ppos); >> +static int proc_sctp_check_rtomax(struct ctl_table *ctl, >> + int write, >> + void __user *buffer, size_t *lenp, >> + loff_t *ppos); >> + >> static struct ctl_table sctp_table[] = { >> { >> .procname = "sctp_mem", >> @@ -102,18 +110,14 @@ static struct ctl_table sctp_net_table[] = { >> .data = &init_net.sctp.rto_min, >> .maxlen = sizeof(unsigned int), >> .mode = 0644, >> - .proc_handler = proc_dointvec_minmax, >> - .extra1 = &one, >> - .extra2 = &timer_max >> + .proc_handler = proc_sctp_check_rtomin, >> }, >> { >> .procname = "rto_max", >> .data = &init_net.sctp.rto_max, >> .maxlen = sizeof(unsigned int), >> .mode = 0644, >> - .proc_handler = proc_dointvec_minmax, >> - .extra1 = &one, >> - .extra2 = &timer_max >> + .proc_handler = proc_sctp_check_rtomax, >> }, >> { >> .procname = "rto_alpha_exp_divisor", >> @@ -342,6 +346,46 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, >> return ret; >> } >> >> +static int proc_sctp_check_rtomin(struct ctl_table *ctl, >> + int write, >> + void __user *buffer, size_t *lenp, >> + loff_t *ppos) >> +{ >> + int old_value = *(int *) ctl->data; >> + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); >> + >> + if (write) { >> + int new_value = *(int *) ctl->data; >> + if (ret || new_value < one || new_value > init_net.sctp.rto_max) { >> + init_net.sctp.rto_min = old_value; >> + return -EINVAL; >> + } >> + init_net.sctp.rto_min = new_value; >> + } >> + >> + return ret; >> +} >> + >> +static int proc_sctp_check_rtomax(struct ctl_table *ctl, >> + int write, >> + void __user*buffer, size_t *lenp, >> + loff_t *ppos) >> +{ >> + int old_value = *(int *) ctl->data; >> + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); >> + >> + if (write) { >> + int new_value = *(int *) ctl->data; >> + if (ret || new_value > timer_max || new_value < init_net.sctp.rto_min) { >> + init_net.sctp.rto_max = old_value; >> + return -EINVAL; >> + } >> + init_net.sctp.rto_max = new_value; >> + } >> + >> + return ret; >> +} >> + > > > You can't used init_net directly. You have to use .data as the target > of assignment and .extra1 and .extra2 as the min and max boundries. > Otherwise, this has the same problem as before where assignments for > created namespace try to change the values in the initial namespace. > > -vlad Hi, Vlad Do You mean like below? For rto_max: .procname = "rto_max", .data = &init_net.sctp.rto_max, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_sctp_check_rtomax, .extra1 = &init_net.sctp.rto_min, .extra2 = &timer_max proc_sctp_check_rtomax() *net = current->nsproxy->net_ns; min = *(unsigned int *) ctl->extra1; max = *(unsigned int *) ctl->extra2; if (write) tbl.data = &new_value; else tbl.data = &net->sctp.rto_max; ret = proc_dointvec(&tbl, write, buffer, lenp, ppos); if (write) { if (ret || new_value > max || new_value < min) { return -EINVAL; } net->sctp.rto_max = new_value; } and as the same as rto_min. Thanks! >> int sctp_sysctl_net_register(struct net *net) >> { >> struct ctl_table *table; >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-sctp" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > > . > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] sctp: check the rto_min and rto_max 2013-12-04 2:46 ` Wang Weidong @ 2013-12-04 14:24 ` Vlad Yasevich 0 siblings, 0 replies; 8+ messages in thread From: Vlad Yasevich @ 2013-12-04 14:24 UTC (permalink / raw) To: Wang Weidong, nhorman, David Miller; +Cc: linux-sctp, netdev On 12/03/2013 09:46 PM, Wang Weidong wrote: > On 2013/12/3 20:39, Vlad Yasevich wrote: >> On 12/03/2013 01:28 AM, Wang Weidong wrote: >>> rto_min should be smaller than rto_max while rto_max should be larger >>> than rto_min. Add two proc_handler for the checking. Add the check in >>> sctp_setsockopt_rtoinfo. >>> >>> Suggested-by: Vlad Yasevich <vyasevich@gmail.com> >>> Signed-off-by: Wang Weidong <wangweidong1@huawei.com> >>> --- >>> net/sctp/socket.c | 5 +++++ >>> net/sctp/sysctl.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++------- >>> 2 files changed, 56 insertions(+), 7 deletions(-) >>> >>> diff --git a/net/sctp/socket.c b/net/sctp/socket.c >>> index 72046b9..2e1af1b 100644 >>> --- a/net/sctp/socket.c >>> +++ b/net/sctp/socket.c >>> @@ -2818,6 +2818,11 @@ static int sctp_setsockopt_rtoinfo(struct sock *sk, char __user *optval, unsigne >>> if (copy_from_user(&rtoinfo, optval, optlen)) >>> return -EFAULT; >>> >>> + if (rtoinfo.srto_min < 1 || >>> + rtoinfo.srto_max > 86400000 || >>> + rtoinfo.srto_max < rtoinfo.srto_min) >>> + return -EINVAL; >>> + >>> asoc = sctp_id2assoc(sk, rtoinfo.srto_assoc_id); >>> >>> /* Set the values to the specific association */ >>> diff --git a/net/sctp/sysctl.c b/net/sctp/sysctl.c >>> index 6b36561..525b06d 100644 >>> --- a/net/sctp/sysctl.c >>> +++ b/net/sctp/sysctl.c >>> @@ -59,8 +59,16 @@ extern int sysctl_sctp_wmem[3]; >>> static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, >>> int write, >>> void __user *buffer, size_t *lenp, >>> - >>> loff_t *ppos); >>> +static int proc_sctp_check_rtomin(struct ctl_table *ctl, >>> + int write, >>> + void __user *buffer, size_t *lenp, >>> + loff_t *ppos); >>> +static int proc_sctp_check_rtomax(struct ctl_table *ctl, >>> + int write, >>> + void __user *buffer, size_t *lenp, >>> + loff_t *ppos); >>> + >>> static struct ctl_table sctp_table[] = { >>> { >>> .procname = "sctp_mem", >>> @@ -102,18 +110,14 @@ static struct ctl_table sctp_net_table[] = { >>> .data = &init_net.sctp.rto_min, >>> .maxlen = sizeof(unsigned int), >>> .mode = 0644, >>> - .proc_handler = proc_dointvec_minmax, >>> - .extra1 = &one, >>> - .extra2 = &timer_max >>> + .proc_handler = proc_sctp_check_rtomin, >>> }, >>> { >>> .procname = "rto_max", >>> .data = &init_net.sctp.rto_max, >>> .maxlen = sizeof(unsigned int), >>> .mode = 0644, >>> - .proc_handler = proc_dointvec_minmax, >>> - .extra1 = &one, >>> - .extra2 = &timer_max >>> + .proc_handler = proc_sctp_check_rtomax, >>> }, >>> { >>> .procname = "rto_alpha_exp_divisor", >>> @@ -342,6 +346,46 @@ static int proc_sctp_do_hmac_alg(struct ctl_table *ctl, >>> return ret; >>> } >>> >>> +static int proc_sctp_check_rtomin(struct ctl_table *ctl, >>> + int write, >>> + void __user *buffer, size_t *lenp, >>> + loff_t *ppos) >>> +{ >>> + int old_value = *(int *) ctl->data; >>> + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); >>> + >>> + if (write) { >>> + int new_value = *(int *) ctl->data; >>> + if (ret || new_value < one || new_value > init_net.sctp.rto_max) { >>> + init_net.sctp.rto_min = old_value; >>> + return -EINVAL; >>> + } >>> + init_net.sctp.rto_min = new_value; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +static int proc_sctp_check_rtomax(struct ctl_table *ctl, >>> + int write, >>> + void __user*buffer, size_t *lenp, >>> + loff_t *ppos) >>> +{ >>> + int old_value = *(int *) ctl->data; >>> + int ret = proc_dointvec(ctl, write, buffer, lenp, ppos); >>> + >>> + if (write) { >>> + int new_value = *(int *) ctl->data; >>> + if (ret || new_value > timer_max || new_value < init_net.sctp.rto_min) { >>> + init_net.sctp.rto_max = old_value; >>> + return -EINVAL; >>> + } >>> + init_net.sctp.rto_max = new_value; >>> + } >>> + >>> + return ret; >>> +} >>> + >> >> >> You can't used init_net directly. You have to use .data as the target >> of assignment and .extra1 and .extra2 as the min and max boundries. >> Otherwise, this has the same problem as before where assignments for >> created namespace try to change the values in the initial namespace. >> >> -vlad > > Hi, Vlad > > Do You mean like below? > > For rto_max: > .procname = "rto_max", > .data = &init_net.sctp.rto_max, > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = &proc_sctp_check_rtomax, > .extra1 = &init_net.sctp.rto_min, > .extra2 = &timer_max > > proc_sctp_check_rtomax() > *net = current->nsproxy->net_ns; > min = *(unsigned int *) ctl->extra1; > max = *(unsigned int *) ctl->extra2; > if (write) > tbl.data = &new_value; > else > tbl.data = &net->sctp.rto_max; > ret = proc_dointvec(&tbl, write, buffer, lenp, ppos); > if (write) { > if (ret || new_value > max || new_value < min) { > return -EINVAL; > } > net->sctp.rto_max = new_value; > } > > and as the same as rto_min. > Thanks! > That looks about right. -vlad >>> int sctp_sysctl_net_register(struct net *net) >>> { >>> struct ctl_table *table; >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> . >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-12-04 14:24 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-12-02 1:45 [PATCH v2] sctp: check the rto_min and rto_max Wang Weidong 2013-12-02 12:15 ` Wang Weidong 2013-12-02 14:45 ` Vlad Yasevich 2013-12-03 1:58 ` Wang Weidong 2013-12-03 6:28 ` [PATCH v3] " Wang Weidong 2013-12-03 12:39 ` Vlad Yasevich 2013-12-04 2:46 ` Wang Weidong 2013-12-04 14:24 ` Vlad Yasevich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).