Date: Tue, 24 Jul 2001 08:13:37 +0100 From: Ben Smithurst <ben@FreeBSD.org> To: Kris Kennaway <kris@obsecurity.org> Cc: audit@FreeBSD.org Subject: Re: rwhod signal fixes Message-ID: <20010724081337.D14233@strontium.shef.vinosystems.com> In-Reply-To: <20010723234000.A2691@xor.obsecurity.org> References: <20010723223904.A1381@xor.obsecurity.org> <20010724072952.A14233@strontium.shef.vinosystems.com> <20010723234000.A2691@xor.obsecurity.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--sfyO1m2EN8ZOtJL6
Content-Type: multipart/mixed; boundary="d01dLTUuW90fS44H"
Content-Disposition: inline
--d01dLTUuW90fS44H
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
Kris Kennaway wrote:
> On Tue, Jul 24, 2001 at 07:29:53AM +0100, Ben Smithurst wrote:
>> Kris Kennaway wrote:
>>=20
>>> + n =3D poll(pfd, 1, 1000);
>>=20
>> Is it really necessary to have rwhod go round in a loop like this
>> wasting CPU time? What was wrong with using alarm() ?
>=20
> It was already using alarm() to wake up every second, but it was doing
> unsafe work in the signal handler.
As far as I could see it was using alarm() to wake up every three minutes,
which is quite different...
What do you think of the attached patch which (I hope) makes it use
SIGALRM in a safe way? It's basically mostly the same as your patch.
I've attached a diff -b version as well since it's easier to see what's
actually changed there...
--=20
Ben Smithurst / ben@FreeBSD.org FreeBSD: The Power To Serve
http://www.FreeBSD.org/
--d01dLTUuW90fS44H
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="rwhod.diff"
Content-Transfer-Encoding: quoted-printable
Index: rwhod.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/cvs/src/usr.sbin/rwhod/rwhod.c,v
retrieving revision 1.15
diff -u -r1.15 rwhod.c
--- rwhod.c 2000/12/22 21:30:15 1.15
+++ rwhod.c 2001/07/24 07:03:37
@@ -149,16 +149,21 @@
struct servent *sp;
int s, utmpf;
=20
+sig_atomic_t gothup, gotalrm;
+
#define WHDRSIZE (sizeof(mywd) - sizeof(mywd.wd_we))
=20
-void run_as __P((uid_t *, gid_t *));
+void alrm __P((int));
int configure __P((int));
-void getboottime __P((int));
-void onalrm __P((int));
+void getboottime __P((void));
+void handleread __P((int s));
+void hup __P((int));
void quit __P((char *));
void rt_xaddrs __P((caddr_t, caddr_t, struct rt_addrinfo *));
-int verify __P((char *, int));
+void run_as __P((uid_t *, gid_t *));
+void timer __P((void));
static void usage __P((void));
+int verify __P((char *, int));
#ifdef DEBUG
char *interval __P((int, char *));
void Sendto __P((int, const void *, size_t, int,
@@ -166,17 +171,28 @@
#define sendto Sendto
#endif
=20
+void
+hup(signo)
+ int signo;
+{
+ gothup =3D 1;
+}
+
+void
+alrm(int signo) {
+ gotalrm =3D 1;
+}
+
int
main(argc, argv)
int argc;
char *argv[];
{
- struct sockaddr_in from;
- struct stat st;
- char path[64];
+ fd_set rset;
int on =3D 1;
char *cp;
struct sockaddr_in sin;
+ struct itimerval it;
uid_t unpriv_uid;
gid_t unpriv_gid;
=20
@@ -213,7 +229,8 @@
#ifndef DEBUG
daemon(1, 0);
#endif
- (void) signal(SIGHUP, getboottime);
+ (void) signal(SIGHUP, hup);
+ (void) signal(SIGALRM, alrm);
openlog("rwhod", LOG_PID, LOG_DAEMON);
sp =3D getservbyname("who", "udp");
if (sp =3D=3D NULL) {
@@ -240,7 +257,7 @@
syslog(LOG_ERR, "%s: %m", _PATH_UTMP);
exit(1);
}
- getboottime(0);
+ getboottime();
if ((s =3D socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
syslog(LOG_ERR, "socket: %m");
exit(1);
@@ -263,74 +280,104 @@
if (!configure(s))
exit(1);
if (!quiet_mode) {
- signal(SIGALRM, onalrm);
- onalrm(0);
+ it.it_value.tv_sec =3D it.it_interval.tv_sec =3D AL_INTERVAL;
+ it.it_value.tv_usec =3D it.it_interval.tv_usec =3D 0;
+ setitimer(ITIMER_REAL, &it, NULL);
+ timer();
}
+
for (;;) {
- struct whod wd;
- int cc, whod, len =3D sizeof(from);
+ int n;
=20
- cc =3D recvfrom(s, (char *)&wd, sizeof(struct whod), 0,
- (struct sockaddr *)&from, &len);
- if (cc <=3D 0) {
- if (cc < 0 && errno !=3D EINTR)
- syslog(LOG_WARNING, "recv: %m");
- continue;
- }
- if (from.sin_port !=3D sp->s_port && !insecure_mode) {
- syslog(LOG_WARNING, "%d: bad source port from %s",
- ntohs(from.sin_port), inet_ntoa(from.sin_addr));
- continue;
- }
- if (cc < WHDRSIZE) {
- syslog(LOG_WARNING, "short packet from %s",
- inet_ntoa(from.sin_addr));
- continue;
- }
- if (wd.wd_vers !=3D WHODVERSION)
- continue;
- if (wd.wd_type !=3D WHODTYPE_STATUS)
- continue;
- if (!verify(wd.wd_hostname, sizeof wd.wd_hostname)) {
- syslog(LOG_WARNING, "malformed host name from %s",
- inet_ntoa(from.sin_addr));
- continue;
- }
- (void) snprintf(path, sizeof path, "whod.%s", wd.wd_hostname);
- /*
- * Rather than truncating and growing the file each time,
- * use ftruncate if size is less than previous size.
- */
- whod =3D open(path, O_WRONLY | O_CREAT, 0644);
- if (whod < 0) {
- syslog(LOG_WARNING, "%s: %m", path);
- continue;
+ FD_ZERO(&rset);
+ FD_SET(s, &rset);
+ n =3D select(s + 1, &rset, NULL, NULL, NULL);
+
+ if (gothup) {
+ gothup =3D 0;
+ getboottime();
+ }
+
+ if (n =3D=3D 1)
+ handleread(s);
+ if (!quiet_mode && gotalrm) {
+ timer();
+ gotalrm =3D 0;
}
+ }
+}
+
+void
+handleread(s)
+ int s;
+{
+ struct sockaddr_in from;
+ struct stat st;
+ char path[64];
+ struct whod wd;
+ int cc, whod, len =3D sizeof(from);
+
+ cc =3D recvfrom(s, (char *)&wd, sizeof(struct whod), 0,
+ (struct sockaddr *)&from, &len);
+ if (cc <=3D 0) {
+ if (cc < 0 && errno !=3D EINTR)
+ syslog(LOG_WARNING, "recv: %m");
+ return;
+ }
+ if (from.sin_port !=3D sp->s_port && !insecure_mode) {
+ syslog(LOG_WARNING, "%d: bad source port from %s",
+ ntohs(from.sin_port), inet_ntoa(from.sin_addr));
+ return;
+ }
+ if (cc < WHDRSIZE) {
+ syslog(LOG_WARNING, "short packet from %s",
+ inet_ntoa(from.sin_addr));
+ return;
+ }
+ if (wd.wd_vers !=3D WHODVERSION)
+ return;
+ if (wd.wd_type !=3D WHODTYPE_STATUS)
+ return;
+ wd.wd_hostname[sizeof(wd.wd_hostname)-1] =3D '\0';
+ if (!verify(wd.wd_hostname, sizeof wd.wd_hostname)) {
+ syslog(LOG_WARNING, "malformed host name from %s",
+ inet_ntoa(from.sin_addr));
+ return;
+ }
+ (void) snprintf(path, sizeof path, "whod.%s", wd.wd_hostname);
+ /*
+ * Rather than truncating and growing the file each time,
+ * use ftruncate if size is less than previous size.
+ */
+ whod =3D open(path, O_WRONLY | O_CREAT, 0644);
+ if (whod < 0) {
+ syslog(LOG_WARNING, "%s: %m", path);
+ return;
+ }
#if ENDIAN !=3D BIG_ENDIAN
- {
- int i, n =3D (cc - WHDRSIZE)/sizeof(struct whoent);
- struct whoent *we;
-
- /* undo header byte swapping before writing to file */
- wd.wd_sendtime =3D ntohl(wd.wd_sendtime);
- for (i =3D 0; i < 3; i++)
- wd.wd_loadav[i] =3D ntohl(wd.wd_loadav[i]);
- wd.wd_boottime =3D ntohl(wd.wd_boottime);
- we =3D wd.wd_we;
- for (i =3D 0; i < n; i++) {
- we->we_idle =3D ntohl(we->we_idle);
- we->we_utmp.out_time =3D
- ntohl(we->we_utmp.out_time);
- we++;
- }
+ {
+ int i, n =3D (cc - WHDRSIZE)/sizeof(struct whoent);
+ struct whoent *we;
+
+ /* undo header byte swapping before writing to file */
+ wd.wd_sendtime =3D ntohl(wd.wd_sendtime);
+ for (i =3D 0; i < 3; i++)
+ wd.wd_loadav[i] =3D ntohl(wd.wd_loadav[i]);
+ wd.wd_boottime =3D ntohl(wd.wd_boottime);
+ we =3D wd.wd_we;
+ for (i =3D 0; i < n; i++) {
+ we->we_idle =3D ntohl(we->we_idle);
+ we->we_utmp.out_time =3D
+ ntohl(we->we_utmp.out_time);
+ we++;
}
-#endif
- (void) time((time_t *)&wd.wd_recvtime);
- (void) write(whod, (char *)&wd, cc);
- if (fstat(whod, &st) < 0 || st.st_size > cc)
- ftruncate(whod, cc);
- (void) close(whod);
}
+#endif
+ (void) time((time_t *)&wd.wd_recvtime);
+ (void) write(whod, (char *)&wd, cc);
+ if (fstat(whod, &st) < 0 || st.st_size > cc)
+ ftruncate(whod, cc);
+ (void) close(whod);
}
=20
static void
@@ -391,8 +438,7 @@
int alarmcount;
=20
void
-onalrm(signo)
- int signo;
+timer()
{
register struct neighbor *np;
register struct whoent *we =3D mywd.wd_we, *wlast;
@@ -404,7 +450,7 @@
=20
now =3D time(NULL);
if (alarmcount % 10 =3D=3D 0)
- getboottime(0);
+ getboottime();
alarmcount++;
(void) fstat(utmpf, &stb);
if ((stb.st_mtime !=3D utmptime) || (stb.st_size > utmpsize)) {
@@ -418,14 +464,14 @@
if (! utmp) {
syslog(LOG_WARNING, "malloc failed");
utmpsize =3D 0;
- goto done;
+ return;
}
}
(void) lseek(utmpf, (off_t)0, L_SET);
cc =3D read(utmpf, (char *)utmp, stb.st_size);
if (cc < 0) {
syslog(LOG_ERR, "read(%s): %m", _PATH_UTMP);
- goto done;
+ return;
}
wlast =3D &mywd.wd_we[1024 / sizeof(struct whoent) - 1];
utmpent =3D cc / sizeof(struct utmp);
@@ -493,13 +539,10 @@
syslog(LOG_ERR, "chdir(%s): %m", _PATH_RWHODIR);
exit(1);
}
-done:
- (void) alarm(AL_INTERVAL);
}
=20
void
-getboottime(signo)
- int signo;
+getboottime()
{
int mib[2];
size_t size;
@@ -691,10 +734,10 @@
register struct whoent *we;
struct sockaddr_in *sin =3D (struct sockaddr_in *)to;
=20
- printf("sendto %x.%d\n", ntohl(sin->sin_addr.s_addr),
- ntohs(sin->sin_port));
+ printf("sendto %s.%d\n", inet_ntoa(sin->sin_addr),
+ ntohs(sin->sin_port));
printf("hostname %s %s\n", w->wd_hostname,
- interval(ntohl(w->wd_sendtime) - ntohl(w->wd_boottime), " up"));
+ interval(ntohl(w->wd_sendtime) - ntohl(w->wd_boottime), " up"));
printf("load %4.2f, %4.2f, %4.2f\n",
ntohl(w->wd_loadav[0]) / 100.0, ntohl(w->wd_loadav[1]) / 100.0,
ntohl(w->wd_loadav[2]) / 100.0);
@@ -702,9 +745,9 @@
for (we =3D w->wd_we, cc /=3D sizeof(struct whoent); cc > 0; cc--, we++) {
time_t t =3D ntohl(we->we_utmp.out_time);
printf("%-8.8s %s:%s %.12s",
- we->we_utmp.out_name,
- w->wd_hostname, we->we_utmp.out_line,
- ctime(&t)+4);
+ we->we_utmp.out_name,
+ w->wd_hostname, we->we_utmp.out_line,
+ ctime(&t)+4);
we->we_idle =3D ntohl(we->we_idle) / 60;
if (we->we_idle) {
if (we->we_idle >=3D 100*60)
@@ -728,18 +771,19 @@
int days, hours, minutes;
=20
if (time < 0 || time > 3*30*24*60*60) {
- (void) sprintf(resbuf, " %s ??:??", updown);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ " %s ??:??", updown);
return (resbuf);
}
minutes =3D (time + 59) / 60; /* round to minutes */
hours =3D minutes / 60; minutes %=3D 60;
days =3D hours / 24; hours %=3D 24;
if (days)
- (void) sprintf(resbuf, "%s %2d+%02d:%02d",
- updown, days, hours, minutes);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ "%s %2d+%02d:%02d", updown, days, hours, minutes);
else
- (void) sprintf(resbuf, "%s %2d:%02d",
- updown, hours, minutes);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ "%s %2d:%02d", updown, hours, minutes);
return (resbuf);
}
#endif
--d01dLTUuW90fS44H
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="rwhod.diff-b"
Content-Transfer-Encoding: quoted-printable
Index: rwhod.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /usr/cvs/src/usr.sbin/rwhod/rwhod.c,v
retrieving revision 1.15
diff -u -b -r1.15 rwhod.c
--- rwhod.c 2000/12/22 21:30:15 1.15
+++ rwhod.c 2001/07/24 07:03:37
@@ -149,16 +149,21 @@
struct servent *sp;
int s, utmpf;
=20
+sig_atomic_t gothup, gotalrm;
+
#define WHDRSIZE (sizeof(mywd) - sizeof(mywd.wd_we))
=20
-void run_as __P((uid_t *, gid_t *));
+void alrm __P((int));
int configure __P((int));
-void getboottime __P((int));
-void onalrm __P((int));
+void getboottime __P((void));
+void handleread __P((int s));
+void hup __P((int));
void quit __P((char *));
void rt_xaddrs __P((caddr_t, caddr_t, struct rt_addrinfo *));
-int verify __P((char *, int));
+void run_as __P((uid_t *, gid_t *));
+void timer __P((void));
static void usage __P((void));
+int verify __P((char *, int));
#ifdef DEBUG
char *interval __P((int, char *));
void Sendto __P((int, const void *, size_t, int,
@@ -166,17 +171,28 @@
#define sendto Sendto
#endif
=20
+void
+hup(signo)
+ int signo;
+{
+ gothup =3D 1;
+}
+
+void
+alrm(int signo) {
+ gotalrm =3D 1;
+}
+
int
main(argc, argv)
int argc;
char *argv[];
{
- struct sockaddr_in from;
- struct stat st;
- char path[64];
+ fd_set rset;
int on =3D 1;
char *cp;
struct sockaddr_in sin;
+ struct itimerval it;
uid_t unpriv_uid;
gid_t unpriv_gid;
=20
@@ -213,7 +229,8 @@
#ifndef DEBUG
daemon(1, 0);
#endif
- (void) signal(SIGHUP, getboottime);
+ (void) signal(SIGHUP, hup);
+ (void) signal(SIGALRM, alrm);
openlog("rwhod", LOG_PID, LOG_DAEMON);
sp =3D getservbyname("who", "udp");
if (sp =3D=3D NULL) {
@@ -240,7 +257,7 @@
syslog(LOG_ERR, "%s: %m", _PATH_UTMP);
exit(1);
}
- getboottime(0);
+ getboottime();
if ((s =3D socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
syslog(LOG_ERR, "socket: %m");
exit(1);
@@ -263,10 +280,40 @@
if (!configure(s))
exit(1);
if (!quiet_mode) {
- signal(SIGALRM, onalrm);
- onalrm(0);
+ it.it_value.tv_sec =3D it.it_interval.tv_sec =3D AL_INTERVAL;
+ it.it_value.tv_usec =3D it.it_interval.tv_usec =3D 0;
+ setitimer(ITIMER_REAL, &it, NULL);
+ timer();
}
+
for (;;) {
+ int n;
+
+ FD_ZERO(&rset);
+ FD_SET(s, &rset);
+ n =3D select(s + 1, &rset, NULL, NULL, NULL);
+
+ if (gothup) {
+ gothup =3D 0;
+ getboottime();
+ }
+
+ if (n =3D=3D 1)
+ handleread(s);
+ if (!quiet_mode && gotalrm) {
+ timer();
+ gotalrm =3D 0;
+ }
+ }
+}
+
+void
+handleread(s)
+ int s;
+{
+ struct sockaddr_in from;
+ struct stat st;
+ char path[64];
struct whod wd;
int cc, whod, len =3D sizeof(from);
=20
@@ -275,26 +322,27 @@
if (cc <=3D 0) {
if (cc < 0 && errno !=3D EINTR)
syslog(LOG_WARNING, "recv: %m");
- continue;
+ return;
}
if (from.sin_port !=3D sp->s_port && !insecure_mode) {
syslog(LOG_WARNING, "%d: bad source port from %s",
ntohs(from.sin_port), inet_ntoa(from.sin_addr));
- continue;
+ return;
}
if (cc < WHDRSIZE) {
syslog(LOG_WARNING, "short packet from %s",
inet_ntoa(from.sin_addr));
- continue;
+ return;
}
if (wd.wd_vers !=3D WHODVERSION)
- continue;
+ return;
if (wd.wd_type !=3D WHODTYPE_STATUS)
- continue;
+ return;
+ wd.wd_hostname[sizeof(wd.wd_hostname)-1] =3D '\0';
if (!verify(wd.wd_hostname, sizeof wd.wd_hostname)) {
syslog(LOG_WARNING, "malformed host name from %s",
inet_ntoa(from.sin_addr));
- continue;
+ return;
}
(void) snprintf(path, sizeof path, "whod.%s", wd.wd_hostname);
/*
@@ -304,7 +352,7 @@
whod =3D open(path, O_WRONLY | O_CREAT, 0644);
if (whod < 0) {
syslog(LOG_WARNING, "%s: %m", path);
- continue;
+ return;
}
#if ENDIAN !=3D BIG_ENDIAN
{
@@ -330,7 +378,6 @@
if (fstat(whod, &st) < 0 || st.st_size > cc)
ftruncate(whod, cc);
(void) close(whod);
- }
}
=20
static void
@@ -391,8 +438,7 @@
int alarmcount;
=20
void
-onalrm(signo)
- int signo;
+timer()
{
register struct neighbor *np;
register struct whoent *we =3D mywd.wd_we, *wlast;
@@ -404,7 +450,7 @@
=20
now =3D time(NULL);
if (alarmcount % 10 =3D=3D 0)
- getboottime(0);
+ getboottime();
alarmcount++;
(void) fstat(utmpf, &stb);
if ((stb.st_mtime !=3D utmptime) || (stb.st_size > utmpsize)) {
@@ -418,14 +464,14 @@
if (! utmp) {
syslog(LOG_WARNING, "malloc failed");
utmpsize =3D 0;
- goto done;
+ return;
}
}
(void) lseek(utmpf, (off_t)0, L_SET);
cc =3D read(utmpf, (char *)utmp, stb.st_size);
if (cc < 0) {
syslog(LOG_ERR, "read(%s): %m", _PATH_UTMP);
- goto done;
+ return;
}
wlast =3D &mywd.wd_we[1024 / sizeof(struct whoent) - 1];
utmpent =3D cc / sizeof(struct utmp);
@@ -493,13 +539,10 @@
syslog(LOG_ERR, "chdir(%s): %m", _PATH_RWHODIR);
exit(1);
}
-done:
- (void) alarm(AL_INTERVAL);
}
=20
void
-getboottime(signo)
- int signo;
+getboottime()
{
int mib[2];
size_t size;
@@ -691,7 +734,7 @@
register struct whoent *we;
struct sockaddr_in *sin =3D (struct sockaddr_in *)to;
=20
- printf("sendto %x.%d\n", ntohl(sin->sin_addr.s_addr),
+ printf("sendto %s.%d\n", inet_ntoa(sin->sin_addr),
ntohs(sin->sin_port));
printf("hostname %s %s\n", w->wd_hostname,
interval(ntohl(w->wd_sendtime) - ntohl(w->wd_boottime), " up"));
@@ -728,18 +771,19 @@
int days, hours, minutes;
=20
if (time < 0 || time > 3*30*24*60*60) {
- (void) sprintf(resbuf, " %s ??:??", updown);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ " %s ??:??", updown);
return (resbuf);
}
minutes =3D (time + 59) / 60; /* round to minutes */
hours =3D minutes / 60; minutes %=3D 60;
days =3D hours / 24; hours %=3D 24;
if (days)
- (void) sprintf(resbuf, "%s %2d+%02d:%02d",
- updown, days, hours, minutes);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ "%s %2d+%02d:%02d", updown, days, hours, minutes);
else
- (void) sprintf(resbuf, "%s %2d:%02d",
- updown, hours, minutes);
+ (void) snprintf(resbuf, sizeof(resbuf),
+ "%s %2d:%02d", updown, hours, minutes);
return (resbuf);
}
#endif
--d01dLTUuW90fS44H--
--sfyO1m2EN8ZOtJL6
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.0.6 (FreeBSD)
Comment: For info see http://www.gnupg.org
iD8DBQE7XSAgbPzJ+yzvRCwRAm50AKC4ahKwoDkLRVCxzFDGKr5bGCAV6QCgy2mP
Y60/9RCV1khtgEyCLV9c41o=
=nVIM
-----END PGP SIGNATURE-----
--sfyO1m2EN8ZOtJL6--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-audit" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010724081337.D14233>