* [PATCH] libipvs: Initialize ipvs_service_t variable @ 2014-01-17 17:53 Ryan O'Hara 2014-01-17 21:03 ` Julian Anastasov 0 siblings, 1 reply; 4+ messages in thread From: Ryan O'Hara @ 2014-01-17 17:53 UTC (permalink / raw) To: lvs-devel; +Cc: Ryan O'Hara The ipvs_get_service function declares an ipvs_service_t type variable and initializes some of the values, but should really start by initializing the entire structure. Signed-off-by: Ryan O'Hara <rohara@redhat.com> --- libipvs/libipvs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c index d2fec49..8baafed 100644 --- a/libipvs/libipvs.c +++ b/libipvs/libipvs.c @@ -942,6 +942,7 @@ ipvs_get_service(__u32 fwmark, __u16 af, __u16 protocol, union nf_inet_addr addr if (!svc) return NULL; + memset(&tsvc, 0, sizeof(tsvc)); tsvc.fwmark = fwmark; tsvc.af = af; tsvc.protocol= protocol; -- 1.8.1.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] libipvs: Initialize ipvs_service_t variable 2014-01-17 17:53 [PATCH] libipvs: Initialize ipvs_service_t variable Ryan O'Hara @ 2014-01-17 21:03 ` Julian Anastasov 2014-01-17 21:15 ` Ryan O'Hara 0 siblings, 1 reply; 4+ messages in thread From: Julian Anastasov @ 2014-01-17 21:03 UTC (permalink / raw) To: Ryan O'Hara; +Cc: lvs-devel Hello, On Fri, 17 Jan 2014, Ryan O'Hara wrote: > The ipvs_get_service function declares an ipvs_service_t type variable > and initializes some of the values, but should really start by > initializing the entire structure. > > Signed-off-by: Ryan O'Hara <rohara@redhat.com> > --- > libipvs/libipvs.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c > index d2fec49..8baafed 100644 > --- a/libipvs/libipvs.c > +++ b/libipvs/libipvs.c > @@ -942,6 +942,7 @@ ipvs_get_service(__u32 fwmark, __u16 af, __u16 protocol, union nf_inet_addr addr > if (!svc) > return NULL; > > + memset(&tsvc, 0, sizeof(tsvc)); > tsvc.fwmark = fwmark; > tsvc.af = af; > tsvc.protocol= protocol; > -- > 1.8.1.4 Yes, it is a good idea. Another variant is to change malloc to calloc. May be ipvs_nl_fill_service_attr() can not crash in all cases when reading svc->pe_name[]. As for the kernel part, ip_vs_genl_parse_service() uses just the initialized fields when full_entry=0 for IPVS_CMD_GET_SERVICE but it is better to avoid problems in the future. Regards -- Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libipvs: Initialize ipvs_service_t variable 2014-01-17 21:03 ` Julian Anastasov @ 2014-01-17 21:15 ` Ryan O'Hara 2014-03-06 10:05 ` Jesper Dangaard Brouer 0 siblings, 1 reply; 4+ messages in thread From: Ryan O'Hara @ 2014-01-17 21:15 UTC (permalink / raw) To: Julian Anastasov; +Cc: lvs-devel On Fri, Jan 17, 2014 at 11:03:43PM +0200, Julian Anastasov wrote: > > Hello, > > On Fri, 17 Jan 2014, Ryan O'Hara wrote: > > > The ipvs_get_service function declares an ipvs_service_t type variable > > and initializes some of the values, but should really start by > > initializing the entire structure. > > > > Signed-off-by: Ryan O'Hara <rohara@redhat.com> > > --- > > libipvs/libipvs.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c > > index d2fec49..8baafed 100644 > > --- a/libipvs/libipvs.c > > +++ b/libipvs/libipvs.c > > @@ -942,6 +942,7 @@ ipvs_get_service(__u32 fwmark, __u16 af, __u16 protocol, union nf_inet_addr addr > > if (!svc) > > return NULL; > > > > + memset(&tsvc, 0, sizeof(tsvc)); > > tsvc.fwmark = fwmark; > > tsvc.af = af; > > tsvc.protocol= protocol; > > -- > > 1.8.1.4 > > Yes, it is a good idea. Another variant is to > change malloc to calloc. May be ipvs_nl_fill_service_attr() > can not crash in all cases when reading svc->pe_name[]. > As for the kernel part, ip_vs_genl_parse_service() uses just > the initialized fields when full_entry=0 for IPVS_CMD_GET_SERVICE > but it is better to avoid problems in the future. Right. This 'bug' was found while running the code through a static analyzer, which finds all sorts of nit-picks. Since the variable in question here is on the stack, a simple memset seems sufficient. Sorry for trivial patches, just trying to get rid of the complaints from the analyzer. Ryan > Regards > > -- > Julian Anastasov <ja@ssi.bg> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libipvs: Initialize ipvs_service_t variable 2014-01-17 21:15 ` Ryan O'Hara @ 2014-03-06 10:05 ` Jesper Dangaard Brouer 0 siblings, 0 replies; 4+ messages in thread From: Jesper Dangaard Brouer @ 2014-03-06 10:05 UTC (permalink / raw) To: Ryan O'Hara; +Cc: Julian Anastasov, lvs-devel On Fri, 17 Jan 2014 15:15:17 -0600 "Ryan O'Hara" <rohara@redhat.com> wrote: > On Fri, Jan 17, 2014 at 11:03:43PM +0200, Julian Anastasov wrote: > > > > Hello, > > > > On Fri, 17 Jan 2014, Ryan O'Hara wrote: > > > > > The ipvs_get_service function declares an ipvs_service_t type variable > > > and initializes some of the values, but should really start by > > > initializing the entire structure. > > > > > > Signed-off-by: Ryan O'Hara <rohara@redhat.com> > > > --- > > > libipvs/libipvs.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/libipvs/libipvs.c b/libipvs/libipvs.c > > > index d2fec49..8baafed 100644 > > > --- a/libipvs/libipvs.c > > > +++ b/libipvs/libipvs.c > > > @@ -942,6 +942,7 @@ ipvs_get_service(__u32 fwmark, __u16 af, __u16 protocol, union nf_inet_addr addr > > > if (!svc) > > > return NULL; > > > > > > + memset(&tsvc, 0, sizeof(tsvc)); > > > tsvc.fwmark = fwmark; > > > tsvc.af = af; > > > tsvc.protocol= protocol; > > > -- > > > 1.8.1.4 > > > > Yes, it is a good idea. Another variant is to > > change malloc to calloc. May be ipvs_nl_fill_service_attr() > > can not crash in all cases when reading svc->pe_name[]. > > As for the kernel part, ip_vs_genl_parse_service() uses just > > the initialized fields when full_entry=0 for IPVS_CMD_GET_SERVICE > > but it is better to avoid problems in the future. > > Right. This 'bug' was found while running the code through a static > analyzer, which finds all sorts of nit-picks. Since the variable in > question here is on the stack, a simple memset seems sufficient. Sorry > for trivial patches, just trying to get rid of the complaints from the > analyzer. Thanks Ryan, Applied the patch. More followup patches to address more areas are welcome. Git tree for ipvsadm avail at: https://git.kernel.org/cgit/utils/kernel/ipvsadm/ipvsadm.git/ -- Best regards, Jesper Dangaard Brouer MSc.CS, Sr. Network Kernel Developer at Red Hat Author of http://www.iptv-analyzer.org LinkedIn: http://www.linkedin.com/in/brouer ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-06 10:05 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-17 17:53 [PATCH] libipvs: Initialize ipvs_service_t variable Ryan O'Hara 2014-01-17 21:03 ` Julian Anastasov 2014-01-17 21:15 ` Ryan O'Hara 2014-03-06 10:05 ` Jesper Dangaard Brouer
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.