* Re: [PATCH] netfilter: fix buffer null termination [not found] ` <CAJ3s=NBCvYnRaWcFb==vZqP1_YE6w=uKqbtBvESkCo1sED2CHg@mail.gmail.com> @ 2016-06-14 15:08 ` Pablo Neira Ayuso 2016-06-14 16:22 ` Kishan Sandeep 0 siblings, 1 reply; 3+ messages in thread From: Pablo Neira Ayuso @ 2016-06-14 15:08 UTC (permalink / raw) To: Kishan Sandeep; +Cc: netdev, netfilter-devel Cc'ing netfilter-devel. On Tue, Jun 14, 2016 at 07:39:27PM +0530, Kishan Sandeep wrote: > + netdev > > On Sat, Jun 11, 2016 at 10:18 AM, Kishan Sandeep > <sandeepkishan108@gmail.com> wrote: > > strncpy generally perferable fo non-terminated > > fixed-width strings. For NULL termination strlcpy > > is preferrable. > > > > Signed-off-by: Kishan Sandeep <sandeepkishan108@gmail.com> > > --- > > net/netfilter/xt_repldata.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > > index 8fd3241..a460211 100644 > > --- a/net/netfilter/xt_repldata.h > > +++ b/net/netfilter/xt_repldata.h > > @@ -28,7 +28,7 @@ > > if (tbl == NULL) \ > > return NULL; \ > > term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > > - strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > > + strlcpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ I don't think this is actually fixing anything. Tables in x_tables have a known and fixed name that is defined from the kernel side, that is always smaller that the buffer we have there. So are you observing any real problem from there? Thanks. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: fix buffer null termination 2016-06-14 15:08 ` [PATCH] netfilter: fix buffer null termination Pablo Neira Ayuso @ 2016-06-14 16:22 ` Kishan Sandeep 2016-06-15 12:17 ` Pablo Neira Ayuso 0 siblings, 1 reply; 3+ messages in thread From: Kishan Sandeep @ 2016-06-14 16:22 UTC (permalink / raw) To: Pablo Neira Ayuso; +Cc: netdev, netfilter-devel Hi Pablo, On Tue, Jun 14, 2016 at 8:38 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > Cc'ing netfilter-devel. > > On Tue, Jun 14, 2016 at 07:39:27PM +0530, Kishan Sandeep wrote: >> + netdev >> >> On Sat, Jun 11, 2016 at 10:18 AM, Kishan Sandeep >> <sandeepkishan108@gmail.com> wrote: >> > strncpy generally perferable fo non-terminated >> > fixed-width strings. For NULL termination strlcpy >> > is preferrable. >> > >> > Signed-off-by: Kishan Sandeep <sandeepkishan108@gmail.com> >> > --- >> > net/netfilter/xt_repldata.h | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h >> > index 8fd3241..a460211 100644 >> > --- a/net/netfilter/xt_repldata.h >> > +++ b/net/netfilter/xt_repldata.h >> > @@ -28,7 +28,7 @@ >> > if (tbl == NULL) \ >> > return NULL; \ >> > term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ >> > - strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ >> > + strlcpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > > I don't think this is actually fixing anything. Tables in x_tables > have a known and fixed name that is defined from the kernel side, that > is always smaller that the buffer we have there. So are you observing > any real problem from there? > > Thanks. Not observed any real problem. Here the string is not NULL terminated with the use of strncpy - that is the problem. With the use of strlcpy we can make the string to terminated properly. Thanks! ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: fix buffer null termination 2016-06-14 16:22 ` Kishan Sandeep @ 2016-06-15 12:17 ` Pablo Neira Ayuso 0 siblings, 0 replies; 3+ messages in thread From: Pablo Neira Ayuso @ 2016-06-15 12:17 UTC (permalink / raw) To: Kishan Sandeep; +Cc: netdev, netfilter-devel On Tue, Jun 14, 2016 at 09:52:49PM +0530, Kishan Sandeep wrote: > Hi Pablo, > > On Tue, Jun 14, 2016 at 8:38 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > Cc'ing netfilter-devel. > > > > On Tue, Jun 14, 2016 at 07:39:27PM +0530, Kishan Sandeep wrote: > >> + netdev > >> > >> On Sat, Jun 11, 2016 at 10:18 AM, Kishan Sandeep > >> <sandeepkishan108@gmail.com> wrote: > >> > strncpy generally perferable fo non-terminated > >> > fixed-width strings. For NULL termination strlcpy > >> > is preferrable. > >> > > >> > Signed-off-by: Kishan Sandeep <sandeepkishan108@gmail.com> > >> > --- > >> > net/netfilter/xt_repldata.h | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/net/netfilter/xt_repldata.h b/net/netfilter/xt_repldata.h > >> > index 8fd3241..a460211 100644 > >> > --- a/net/netfilter/xt_repldata.h > >> > +++ b/net/netfilter/xt_repldata.h > >> > @@ -28,7 +28,7 @@ > >> > if (tbl == NULL) \ > >> > return NULL; \ > >> > term = (struct type##_error *)&(((char *)tbl)[term_offset]); \ > >> > - strncpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > >> > + strlcpy(tbl->repl.name, info->name, sizeof(tbl->repl.name)); \ > > > > I don't think this is actually fixing anything. Tables in x_tables > > have a known and fixed name that is defined from the kernel side, that > > is always smaller that the buffer we have there. So are you observing > > any real problem from there? > > > > Thanks. > > Not observed any real problem. Here the string is not NULL terminated with > the use of strncpy - that is the problem. With the use of strlcpy we can make > the string to terminated properly. 1) Table names set from the kernel codebase, so they are always way smaller than that. 2) We're not expecting the addition of new tables in the future that we result in the hypothetical problem that you indicate. Thanks. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-06-15 12:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1465620484-5543-1-git-send-email-sandeepkishan108@gmail.com>
[not found] ` <CAJ3s=NBCvYnRaWcFb==vZqP1_YE6w=uKqbtBvESkCo1sED2CHg@mail.gmail.com>
2016-06-14 15:08 ` [PATCH] netfilter: fix buffer null termination Pablo Neira Ayuso
2016-06-14 16:22 ` Kishan Sandeep
2016-06-15 12:17 ` Pablo Neira Ayuso
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).