Re: dmesg alloc

2019-12-23 Thread Philip Guenther
On Mon, 23 Dec 2019, Alexander Bluhm wrote:
> dmesg(8) allocates a bit too much memory.
> 
> sysctl KERN_MSGBUFSIZE returns msg_bufs.
> sysctl KERN_MSGBUF copies out msg_bufs + offsetof(struct msgbuf,
> msg_bufc) bytes.
> dmesg allocates msg_bufs + sizeof(struct msgbuf) - 1 bytes.
> 
> This is not the same value as struct msgbuf is padded.
> struct msgbuf {
>   ...
> longmsg_bufd;   /* number of dropped bytes */
> charmsg_bufc[1];/* buffer */
> };
> 
> Better use the same size algorithm in kernel and userland.
> 
> ok?

The first chunk should be replaced with this one:

@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 

to just pick up the offsetof() supplied for the last 30 years by the C 
standard.  Otherwise, ok.



dmesg alloc

2019-12-23 Thread Alexander Bluhm
Hi,

dmesg(8) allocates a bit too much memory.

sysctl KERN_MSGBUFSIZE returns msg_bufs.
sysctl KERN_MSGBUF copies out msg_bufs + offsetof(struct msgbuf,
msg_bufc) bytes.
dmesg allocates msg_bufs + sizeof(struct msgbuf) - 1 bytes.

This is not the same value as struct msgbuf is padded.
struct msgbuf {
...
longmsg_bufd;   /* number of dropped bytes */
charmsg_bufc[1];/* buffer */
};

Better use the same size algorithm in kernel and userland.

ok?

bluhm

Index: sbin/dmesg/dmesg.c
===
RCS file: /data/mirror/openbsd/cvs/src/sbin/dmesg/dmesg.c,v
retrieving revision 1.30
diff -u -p -r1.30 dmesg.c
--- sbin/dmesg/dmesg.c  15 May 2018 15:15:50 -  1.30
+++ sbin/dmesg/dmesg.c  23 Dec 2019 22:12:44 -
@@ -55,6 +55,9 @@ struct nlist nl[] = {

 void usage(void);

+#ifndef offsetof
+#define offsetof(s, e) ((size_t)&((s *)0)->e)
+#endif
 #defineKREAD(addr, var) \
kvm_read(kd, addr, , sizeof(var)) != sizeof(var)

@@ -99,7 +102,7 @@ main(int argc, char *argv[])
err(1, "sysctl: %s", startupmsgs ? "KERN_CONSBUFSIZE" :
"KERN_MSGBUFSIZE");

-   msgbufsize += sizeof(struct msgbuf) - 1;
+   msgbufsize += offsetof(struct msgbuf, msg_bufc);
allocated = bufdata = calloc(1, msgbufsize);
if (bufdata == NULL)
errx(1, "couldn't allocate space for buffer data");