On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> "Nathanael D. Noblet" <nathanael@xxxxxxx> writes:
>
> > Hello,
> >
> > I maintain beanstalkd which is a message server of sorts. It recently
> > released a new version however some changes I'm not 100% sure about.
> >
> > When compiling I got the following GCC error.
> >
> > usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy'
> > specified bound 201 equals destination size [-Werror=stringop-
> > truncation]
> > 106 | return __builtin___strncpy_chk (__dest, __src, __len, __bos
> > (__dest));
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ~~~~~~~~
> >
> > Results via google says that strncpy needs to have the third argument
> > less than the 2nd, so I've got this patch, similar to others:
> >
> > --- beanstalkd-1.12/tube.c.org 2020-06-10 09:37:35.129224346 -0600
> > +++ beanstalkd-1.12/tube.c 2020-06-10 09:37:42.761138035 -0600
> > @@ -12,7 +12,7 @@
> > if (!t)
> > return NULL;
> >
> > - strncpy(t->name, name, MAX_TUBE_NAME_LEN);
> > + strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
> > if (t->name[MAX_TUBE_NAME_LEN - 1] != '\0') {
> > t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
> > twarnx("truncating tube name");
> >
> > You'll notice it was already checking the len-1 for null. Can anyone
> > verify that my change won't cause some un-intended bug I don't
> > understand?
>
> If I understand it correctly, then you are now invoking undefined
> behavior: you copy at most MAX_TUBE_NAME_LEN-1 bytes to t->name and then
> check whether the following byte (that was never written to) is not
> equal to 0. I have not checked the code of beanstalkd, but the contents
> of t->name[MAX_TUBE_NAME_LEN - 1] will be zero if it was allocated via
> calloc() at best or random at worst. Irrespectively of that, the check
> now no longer does what it *should*: null terminate the string if it is
> not null terminated. It will now randomly null terminate it or not at
> all.
strncpy() is a truly awful API unfortunately, the change is
meaningless, but it is not random as Dan says.
The original form is more correct though, because now you can get
spurious warnings that the string have been truncated when that is not
true. (If t->name is not zeroized you will get the spurious warning for
any string shorter than MAX_TUBE_NAME_LEN, which is the opposite of
what the warning says.
The third argument to strncpy can be as long as the size of the buffer
you are writing into, the additional check you have there insures that
the string is terminated (even if that requires truncation).
So I would say you should drop your change and stop believing in random
google results :)
> > Finally, they also now support socket activation. Which means I was
> > looking at .service file which I had setup to use a sysconfig
> > environment file. When I looked at it, I realized that the default was
> > to listen to 0.0.0.0 which means someone could install and if not
> > really careful / pre config or what not have a service listening where
> > they maybe don't want to. I was thinking of changing this to 127.0.0.1
> > with a note about how it should be modified. However I figure if I do
> > that, anyone with this installed and unmodified who gets the update
> > will suddenly have their service stop listening to the outside world.
> > What should be done then?
> >
> > My thought is I leave it as 0.0.0.0 for all released versions (but what
> > about epel7) and then in rawhide change to 127.0.0.1. I just don't like
> > leaving the package like that for epel as well.
> >
> > Any ideas/direction would be appreciated.
> >
> > Sincerely,
> > --
> > Nathanael
> >
> > _______________________________________________
> > devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
> > To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
> > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> > List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
>
> _______________________________________________
> devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
> Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc
_______________________________________________
devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx
