Re: fix segfault on radiusd(8)
:facepalm: let's forget I sent the previous patch.
OK?
Index: radiusd.c
===
RCS file: /cvs/src/usr.sbin/radiusd/radiusd.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 radiusd.c
--- radiusd.c 13 Jun 2017 05:40:22 - 1.20
+++ radiusd.c 1 Aug 2018 12:26:26 -
@@ -1066,7 +1066,8 @@ radiusd_module_stop(struct radiusd_modul
{
module->stopped = true;
- freezero(module->secret, strlen(module->secret));
+ if (module->secret != NULL)
+ freezero(module->secret, strlen(module->secret));
module->secret = NULL;
if (module->fd >= 0) {
On 14:12 Wed 01 Aug , Sebastien Marie wrote:
> if I didn't mess, module is a `struct radiusd_module', and `secret'
> member is defined as `char *'. I expect sizeof(module->secret) to be
> always 8 (on amd64): the size of the pointer.
>
> as the variable is dynamically allocated, you should really use strlen()
> and checking for NULL before zeroing it.
>
> --
> Sebastien Marie
Re: fix segfault on radiusd(8)
On Wed, Aug 01, 2018 at 12:54:49PM +0100, Ricardo Mestre wrote:
> Hi,
>
> When radiusd(8) starts shutting down one of the actions is to iterate through
> the configured modules and freezero(3) each module->secret. By using the
> config
> from /etc/examples/radiusd.conf which has module bsdauth and radius configured
> and running it with -d then hit ^C it will segfault. This is due to bsdauth
> module not having a secret and therefore when calling strlen(NULL) it just
> segfaults since this is not a valid argument.
>
> In order to avoid this issue we just have to swap strlen(3) with sizeof().
>
> OK?
if I didn't mess, module is a `struct radiusd_module', and `secret'
member is defined as `char *'. I expect sizeof(module->secret) to be
always 8 (on amd64): the size of the pointer.
as the variable is dynamically allocated, you should really use strlen()
and checking for NULL before zeroing it.
--
Sebastien Marie
> Index: radiusd.c
> ===
> RCS file: /cvs/src/usr.sbin/radiusd/radiusd.c,v
> retrieving revision 1.20
> diff -u -p -u -r1.20 radiusd.c
> --- radiusd.c 13 Jun 2017 05:40:22 - 1.20
> +++ radiusd.c 1 Aug 2018 11:46:43 -
> @@ -1066,7 +1066,7 @@ radiusd_module_stop(struct radiusd_modul
> {
> module->stopped = true;
>
> - freezero(module->secret, strlen(module->secret));
> + freezero(module->secret, sizeof(module->secret));
> module->secret = NULL;
>
> if (module->fd >= 0) {
>
Re: fix segfault on radiusd(8)
Forgot to mention that I didn't use an if (module->secret != NULL) since
it may look like the problem would be with freezero(3) which is not the
case because this one accepts a NULL pointer without issues.
On 12:54 Wed 01 Aug , Ricardo Mestre wrote:
> Hi,
>
> When radiusd(8) starts shutting down one of the actions is to iterate through
> the configured modules and freezero(3) each module->secret. By using the
> config
> from /etc/examples/radiusd.conf which has module bsdauth and radius configured
> and running it with -d then hit ^C it will segfault. This is due to bsdauth
> module not having a secret and therefore when calling strlen(NULL) it just
> segfaults since this is not a valid argument.
>
> In order to avoid this issue we just have to swap strlen(3) with sizeof().
>
> OK?
>
> Index: radiusd.c
> ===
> RCS file: /cvs/src/usr.sbin/radiusd/radiusd.c,v
> retrieving revision 1.20
> diff -u -p -u -r1.20 radiusd.c
> --- radiusd.c 13 Jun 2017 05:40:22 - 1.20
> +++ radiusd.c 1 Aug 2018 11:46:43 -
> @@ -1066,7 +1066,7 @@ radiusd_module_stop(struct radiusd_modul
> {
> module->stopped = true;
>
> - freezero(module->secret, strlen(module->secret));
> + freezero(module->secret, sizeof(module->secret));
> module->secret = NULL;
>
> if (module->fd >= 0) {
>
fix segfault on radiusd(8)
Hi,
When radiusd(8) starts shutting down one of the actions is to iterate through
the configured modules and freezero(3) each module->secret. By using the config
from /etc/examples/radiusd.conf which has module bsdauth and radius configured
and running it with -d then hit ^C it will segfault. This is due to bsdauth
module not having a secret and therefore when calling strlen(NULL) it just
segfaults since this is not a valid argument.
In order to avoid this issue we just have to swap strlen(3) with sizeof().
OK?
Index: radiusd.c
===
RCS file: /cvs/src/usr.sbin/radiusd/radiusd.c,v
retrieving revision 1.20
diff -u -p -u -r1.20 radiusd.c
--- radiusd.c 13 Jun 2017 05:40:22 - 1.20
+++ radiusd.c 1 Aug 2018 11:46:43 -
@@ -1066,7 +1066,7 @@ radiusd_module_stop(struct radiusd_modul
{
module->stopped = true;
- freezero(module->secret, strlen(module->secret));
+ freezero(module->secret, sizeof(module->secret));
module->secret = NULL;
if (module->fd >= 0) {
