Re: A few questions about a package update / policy questions / GCC 9 error
On Thu, 2020-06-11 at 16:13 -0400, Simo Sorce wrote: > I wish this was easier done than said :) Yeah, this isn't *my* project, I just packaged it. Thank you to everyone who added suggestions to dealing with the C code. I'll take a look and see what upstream thinks about it. I am wondering if anyone has thoughts about changing the sysconfig file. Sincerely, -- Nathanael ___ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org 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@lists.fedoraproject.org
Re: A few questions about a package update / policy questions / GCC 9 error
On Thu, 2020-06-11 at 20:21 +0200, Björn Persson wrote:
> Simo Sorce wrote:
> > If you really want to avoid the warning instead of ignoring it, you
> > should change the code this way:
> >
> > strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
> > if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
> > t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
> > twarnx("truncating tube name");
> > }
>
> That code will warn wrongly about truncation when the name is exactly
> MAX_TUBE_NAME_LEN - 1 bytes long.
Yes but that is unavoidable unless you ant to add a check on the source
length, which is entirely doable of course.
> Nathanael, you could link to libbsd and use strlcpy if that's available
> on all target platforms. Or, if you happen to be using Glib, use
> g_strlcpy. Otherwise use Peter's snprintf solution, but DO NOT make the
> mistake of passing the input as the format string!
>
> And choose a better language than C for your next project.
I wish this was easier done than said :)
Simo.
--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Re: A few questions about a package update / policy questions / GCC 9 error
Simo Sorce wrote:
> If you really want to avoid the warning instead of ignoring it, you
> should change the code this way:
>
> strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
> if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
> t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
> twarnx("truncating tube name");
> }
That code will warn wrongly about truncation when the name is exactly
MAX_TUBE_NAME_LEN - 1 bytes long.
Nathanael, you could link to libbsd and use strlcpy if that's available
on all target platforms. Or, if you happen to be using Glib, use
g_strlcpy. Otherwise use Peter's snprintf solution, but DO NOT make the
mistake of passing the input as the format string!
And choose a better language than C for your next project.
Björn Persson
pgpUai9i5B_si.pgp
Description: OpenPGP digital signatur
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Re: A few questions about a package update / policy questions / GCC 9 error
On 10/06/20 21:43 +0300, Peter Pentchev wrote:
On Wed, Jun 10, 2020 at 02:22:53PM -0400, Simo Sorce wrote:
On Wed, 2020-06-10 at 14:16 -0400, Simo Sorce wrote:
> On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> > "Nathanael D. Noblet" 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 :)
Sorry, hit send prematurely.
If you really want to avoid the warning instead of ignoring it, you
should change the code this way:
strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
twarnx("truncating tube name");
}
NOTE: the -2 in the condition, this is needed because memory locations
start counting from 0, so if you write N-1 bytes the Nth-1 byte is at
the N-2 position when you start counting from 0.
And that is the last position your strncpy wrote to, and if that is not
0 then potentially string truncation occurred.
You still want to zero the last available byte in that case and not the
N-1 available byte, so you set N-1 to 0, not N-2.
N-1 is the Nth byte when you start counting from 0 and N is the size of
the array.
...or use snprintf() and check! its! return! value! (sorry, I've had to
explain that to too many people over too many years).
^ This.
Something like (completely, totally untested, but should give you an
idea):
const int res = snprintf(t->name, MAX_TUBE_NAME_LEN, "%s", name);
if (res < 0 || (size_t)res >= MAX_TUBE_NAME_LEN)
twarnx("truncating tube name");
...although, in all fairness, the two conditions should be checked
separately, since the first one (res < 0) indicates a serious error and
no meaningful characters at all written to t->name.
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
Re: A few questions about a package update / policy questions / GCC 9 error
On 20-06-10 14:22:53, Simo Sorce wrote:
On Wed, 2020-06-10 at 14:16 -0400, Simo Sorce wrote:
> On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> > "Nathanael D. Noblet" 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 :)
Sorry, hit send prematurely.
If you really want to avoid the warning instead of ignoring it, you
should change the code this way:
strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
twarnx("truncating tube name");
}
NOTE: the -2 in the condition, this is needed because memory locations
start counting from 0, so if you write N-1 bytes the Nth-1 byte is at
the N-2 position when you start counting from 0.
And that is the last position your strncpy wrote to, and if that is
not
0 then potentially string truncation occurred.
You still want to zero the last available byte in that case and not
the
N-1 available byte, so you set N-1 to 0, not N-2.
N-1 is the Nth byte when you start counting from 0 and N is the size
of
the array.
HTH,
Simo.
I prefer to not look outside of the string's data into apparently
uninitialized memory. Was t->name initialized elsewhere? I would
write:
t->name[0] = 0;
strncat(t->name, name, MAX_TUBE_NAME_LEN-1);
if (strcmp(t->name, name)) {
...
strncat always writes a terminating NUL.
strcmp only looks at the string data, not past its end.
BTW, a better name for MAX_TUBE_NAME_LEN would be MAX_TUBE_NAME_SIZ.
C, bah.
--
TonyN.:'
'
Re: A few questions about a package update / policy questions / GCC 9 error
On Wed, Jun 10, 2020 at 02:22:53PM -0400, Simo Sorce wrote:
> On Wed, 2020-06-10 at 14:16 -0400, Simo Sorce wrote:
> > On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> > > "Nathanael D. Noblet" 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 :)
>
> Sorry, hit send prematurely.
>
> If you really want to avoid the warning instead of ignoring it, you
> should change the code this way:
>
> strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
> if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
> t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
> twarnx("truncating tube name");
> }
>
> NOTE: the -2 in the condition, this is needed because memory locations
> start counting from 0, so if you write N-1 bytes the Nth-1 byte is at
> the N-2 position when you start counting from 0.
> And that is the last position your strncpy wrote to, and if that is not
> 0 then potentially string truncation occurred.
>
> You still want to zero the last available byte in that case and not the
> N-1 available byte, so you set N-1 to 0, not N-2.
>
> N-1 is the Nth byte when you start counting from 0 and N is the size of
> the array.
...or use snprintf() and check! its! return! value! (sorry, I've had to
explain that to too many people over too many years).
Something like (completely, totally untested, but should give you an
idea):
const int res = snprintf(t->name, MAX_TUBE_NAME_LEN, "%s", name);
if (res < 0 || (size_t)res >= MAX_TUBE_NAME_LEN)
twarnx("truncating tube name");
...although, in all fairness, the two conditions should be checked
separately, since the first one (res < 0) indicates a serious error and
no meaningful characters at all written to t->name.
G'luck,
Peter
--
Peter Pentchev r...@ringlet.net r...@debian.org p...@storpool.com
PGP key:http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint 2EE7 A7A5 17FC 124C F115 C354 651E EFB0 2527 DF13
signature.asc
Description: PGP signature
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
Re: A few questions about a package update / policy questions / GCC 9 error
On Wed, 2020-06-10 at 14:16 -0400, Simo Sorce wrote:
> On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> > "Nathanael D. Noblet" 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 :)
Sorry, hit send prematurely.
If you really want to avoid the warning instead of ignoring it, you
should change the code this way:
strncpy(t->name, name, MAX_TUBE_NAME_LEN-1);
if (t->name[MAX_TUBE_NAME_LEN - 2] != '\0') {
t->name[MAX_TUBE_NAME_LEN - 1] = '\0';
twarnx("truncating tube name");
}
NOTE: the -2 in the condition, this is needed because memory locations
start counting from 0, so if you write N-1 bytes the Nth-1 byte is at
the N-2 position when you start counting from 0.
And that is the last position your strncpy wrote to, and if that is not
0 then potentially string truncation occurred.
You still want to zero the last available byte in that case and not the
N-1 available byte, so you set N-1 to 0, not N-2.
N-1 is the Nth byte when you start counting from 0 and N is the size of
the array.
HTH,
Simo.
> > > 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@lists.fedoraproject.org
> > > To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> > > Fedora Code of Conduct:
> > >
Re: A few questions about a package update / policy questions / GCC 9 error
On Wed, 2020-06-10 at 20:01 +0200, Dan Čermák wrote:
> "Nathanael D. Noblet" 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@lists.fedoraproject.org
> > To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> > 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@lists.fedoraproject.org
>
> ___
> devel mailing list -- devel@lists.fedoraproject.org
> To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> 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@lists.fedoraproject.org
--
Simo Sorce
RHEL Crypto Team
Red Hat, Inc
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List
Re: A few questions about a package update / policy questions / GCC 9 error
"Nathanael D. Noblet" 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.
>
> 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@lists.fedoraproject.org
> To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> 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@lists.fedoraproject.org
signature.asc
Description: PGP signature
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
A few questions about a package update / policy questions / GCC 9 error
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?
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@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
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@lists.fedoraproject.org
