On Mon, Sep 03, 2018 at 11:43:02PM +0800, Michael Mikonos wrote:
> On Mon, Sep 03, 2018 at 02:24:49PM +0800, Michael Mikonos wrote:
> > On Sat, Sep 01, 2018 at 11:31:49PM +0200, Gilles Chehade wrote:
> > > On Sat, Sep 01, 2018 at 09:20:59PM +0800, Michael Mikonos wrote:
> > > > Hello,
> > > >
> > > > Replace a malloc+strlcpy with strndup in cmdline_symset().
> > > > Parameter s is a "keyname=value" string and sym is the
> > > > "keyname" part.
> > > >
> > > > If s is "=value", sym will be an empty string.
> > > > The patch doesn't change this behaviour although
> > > > it might be undesirable to call symset() with
> > > > an empty string. Possibly it could also return -1
> > > > if len is zero. Thoughts?
> > > >
> > >
> > > Not opposed to the diff but at this late hour I find it easier to read
> > > the malloc+strlcpy and be sure there's not an off-by-one than with the
> > > strndup version, I'll read again tomorrow.
> >
> > In my understanding the length argument of strndup(3) doesn't include
> > the terminating NUL character. I think the linux manual for strndup(3)
> > is slightly clearer on this because it has the text:
> >
> > ... only n bytes are copied, and a terminating null byte ('\0') is
> > added.
> >
> > > Just wanted to remind you that this function is shared between daemons
> > > so this can't be an smtpd-only change :-)
>
> Thanks for the reminder. Here is a new version of the patch to include
> other daemons. I also followed a suggestion from halex@ to remove the
> strlen() calls and determine length using val-s. Did I miss anything?
>
this looks good to me
> Index: acme-client/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/acme-client/parse.y,v
> retrieving revision 1.29
> diff -u -p -u -r1.29 parse.y
> --- acme-client/parse.y 3 Aug 2018 17:57:21 -0000 1.29
> +++ acme-client/parse.y 3 Sep 2018 15:18:23 -0000
> @@ -839,17 +839,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return -1;
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(EXIT_FAILURE, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(EXIT_FAILURE, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: bgpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
> retrieving revision 1.331
> diff -u -p -u -r1.331 parse.y
> --- bgpd/parse.y 27 Aug 2018 19:32:37 -0000 1.331
> +++ bgpd/parse.y 3 Sep 2018 15:18:24 -0000
> @@ -3145,17 +3145,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - fatal("cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + fatal("%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: dvmrpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/dvmrpd/parse.y,v
> retrieving revision 1.36
> diff -u -p -u -r1.36 parse.y
> --- dvmrpd/parse.y 11 Jul 2018 07:39:22 -0000 1.36
> +++ dvmrpd/parse.y 3 Sep 2018 15:18:24 -0000
> @@ -834,17 +834,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: eigrpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/eigrpd/parse.y,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 parse.y
> --- eigrpd/parse.y 11 Jul 2018 07:39:22 -0000 1.27
> +++ eigrpd/parse.y 3 Sep 2018 15:18:24 -0000
> @@ -1094,17 +1094,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: httpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/httpd/parse.y,v
> retrieving revision 1.105
> diff -u -p -u -r1.105 parse.y
> --- httpd/parse.y 11 Jul 2018 07:39:22 -0000 1.105
> +++ httpd/parse.y 3 Sep 2018 15:18:25 -0000
> @@ -1819,17 +1819,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ifstated/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ifstated/parse.y,v
> retrieving revision 1.52
> diff -u -p -u -r1.52 parse.y
> --- ifstated/parse.y 11 Jul 2018 07:39:22 -0000 1.52
> +++ ifstated/parse.y 3 Sep 2018 15:18:25 -0000
> @@ -871,17 +871,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> err(1, "%s", __func__);
> -
> - strlcpy(sym, s, len);
> -
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: iscsictl/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/iscsictl/parse.y,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 parse.y
> --- iscsictl/parse.y 11 Jul 2018 07:39:22 -0000 1.15
> +++ iscsictl/parse.y 3 Sep 2018 15:18:25 -0000
> @@ -739,17 +739,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ldapd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldapd/parse.y,v
> retrieving revision 1.32
> diff -u -p -u -r1.32 parse.y
> --- ldapd/parse.y 11 Jul 2018 07:39:22 -0000 1.32
> +++ ldapd/parse.y 3 Sep 2018 15:18:25 -0000
> @@ -911,17 +911,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - fatal("cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + fatal("%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ldpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ldpd/parse.y,v
> retrieving revision 1.65
> diff -u -p -u -r1.65 parse.y
> --- ldpd/parse.y 11 Jul 2018 07:39:22 -0000 1.65
> +++ ldpd/parse.y 3 Sep 2018 15:18:26 -0000
> @@ -1317,17 +1317,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: lpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/lpd/parse.y,v
> retrieving revision 1.3
> diff -u -p -u -r1.3 parse.y
> --- lpd/parse.y 11 Jul 2018 07:39:22 -0000 1.3
> +++ lpd/parse.y 3 Sep 2018 15:18:26 -0000
> @@ -682,17 +682,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "%s", __func__);
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ospf6d/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospf6d/parse.y,v
> retrieving revision 1.38
> diff -u -p -u -r1.38 parse.y
> --- ospf6d/parse.y 12 Jul 2018 13:45:03 -0000 1.38
> +++ ospf6d/parse.y 3 Sep 2018 15:18:26 -0000
> @@ -1095,17 +1095,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ospfd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ospfd/parse.y,v
> retrieving revision 1.91
> diff -u -p -u -r1.91 parse.y
> --- ospfd/parse.y 11 Jul 2018 07:39:22 -0000 1.91
> +++ ospfd/parse.y 3 Sep 2018 15:18:27 -0000
> @@ -1294,17 +1294,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: rad/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rad/parse.y,v
> retrieving revision 1.8
> diff -u -p -u -r1.8 parse.y
> --- rad/parse.y 3 Aug 2018 13:14:46 -0000 1.8
> +++ rad/parse.y 3 Sep 2018 15:18:27 -0000
> @@ -914,17 +914,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: relayd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
> retrieving revision 1.227
> diff -u -p -u -r1.227 parse.y
> --- relayd/parse.y 6 Aug 2018 17:31:31 -0000 1.227
> +++ relayd/parse.y 3 Sep 2018 15:18:27 -0000
> @@ -2902,17 +2902,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ripd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ripd/parse.y,v
> retrieving revision 1.41
> diff -u -p -u -r1.41 parse.y
> --- ripd/parse.y 11 Jul 2018 07:39:22 -0000 1.41
> +++ ripd/parse.y 3 Sep 2018 15:18:28 -0000
> @@ -850,17 +850,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: smtpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
> retrieving revision 1.219
> diff -u -p -u -r1.219 parse.y
> --- smtpd/parse.y 1 Sep 2018 21:20:32 -0000 1.219
> +++ smtpd/parse.y 3 Sep 2018 15:18:28 -0000
> @@ -2124,17 +2124,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: snmpd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/snmpd/parse.y,v
> retrieving revision 1.51
> diff -u -p -u -r1.51 parse.y
> --- snmpd/parse.y 11 Jul 2018 07:39:22 -0000 1.51
> +++ snmpd/parse.y 3 Sep 2018 15:18:28 -0000
> @@ -1127,17 +1127,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: switchd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/switchd/parse.y,v
> retrieving revision 1.10
> diff -u -p -u -r1.10 parse.y
> --- switchd/parse.y 11 Jul 2018 07:39:22 -0000 1.10
> +++ switchd/parse.y 3 Sep 2018 15:18:28 -0000
> @@ -705,17 +705,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = (val - s) + 1;
> - if ((sym = malloc(len)) == NULL)
> - fatal("cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + fatal("%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: vmd/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> retrieving revision 1.42
> diff -u -p -u -r1.42 parse.y
> --- vmd/parse.y 13 Jul 2018 08:42:49 -0000 1.42
> +++ vmd/parse.y 3 Sep 2018 15:18:28 -0000
> @@ -1154,17 +1154,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = (val - s) + 1;
> - if ((sym = malloc(len)) == NULL)
> - fatal("cmdline_symset: malloc");
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + fatal("%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
> Index: ypldap/parse.y
> ===================================================================
> RCS file: /cvs/src/usr.sbin/ypldap/parse.y,v
> retrieving revision 1.28
> diff -u -p -u -r1.28 parse.y
> --- ypldap/parse.y 11 Jul 2018 07:39:22 -0000 1.28
> +++ ypldap/parse.y 3 Sep 2018 15:18:29 -0000
> @@ -930,17 +930,12 @@ cmdline_symset(char *s)
> {
> char *sym, *val;
> int ret;
> - size_t len;
>
> if ((val = strrchr(s, '=')) == NULL)
> return (-1);
> -
> - len = strlen(s) - strlen(val) + 1;
> - if ((sym = malloc(len)) == NULL)
> - errx(1, "%s", __func__);
> -
> - (void)strlcpy(sym, s, len);
> -
> + sym = strndup(s, val - s);
> + if (sym == NULL)
> + errx(1, "%s: strndup", __func__);
> ret = symset(sym, val + 1, 1);
> free(sym);
>
>
--
Gilles Chehade
https://www.poolp.org @poolpOrg
