Date: Thu, 24 May 2001 21:49:18 +0200 From: Thomas Moestl <tmm@freebsd.org> To: Dima Dorfman <dima@unixfreak.org> Cc: audit@freebsd.org Subject: Re: Patch to remove setgid bit from ipcs(1) Message-ID: <20010524214918.A2640@crow.dom2ip.de> In-Reply-To: <20010524024051.A40E83E8E@bazooka.unixfreak.org>; from dima@unixfreak.org on Wed, May 23, 2001 at 07:40:51PM -0700 References: <20010523175221.C594@crow.dom2ip.de> <20010524024051.A40E83E8E@bazooka.unixfreak.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, 2001/05/23 at 19:40:51 -0700, Dima Dorfman wrote:
> Thomas Moestl <tmm@freebsd.org> writes:
>
> All that said, I think I might've found a happy medium. With a little
> preprocessor magic in ipcs(1), I can make it maintainable without
> having to export the data as a structure. Essentially, the new
> function sysctlgatherstruct() takes a pointer to the structure to fill
> in, the size, and an array of "vectors"; one for every field that
> needs to be filled in. The latter has information on which sysctl to
> use to get the information, the size, and where in the structure it
> should be put.
>
> One thing I don't really like is that the code in the userland is now
> quite a bit bigger (and, since I tried to make sysctlgatherstruct
> generic so it can potentially be used in another program, it's
> probably a little bigger than it could've been), but I think it's
> worth it to lose the setgid bit; once that's done, it is no longer
> much of a threat to security.
Yeah.
I've found a few more things, most by compiling with BDECFLAGS. Most
do not affect functionality in any way, so they are not really
important. I just thought I would mention them...
> I've attached an updated patch. It incorporates all of your
> suggestions, including exporting the individual fields rather than the
> structure as described above.
>
> +SYSCTL_PROC(_kern_ipc, OID_AUTO, msqids, CTLFLAG_ANYBODY | CTLFLAG_RD,
> + NULL, 0, sysctl_msqids, "", "Message queue IDs");
<nitpick>
There are some occurances of CTLFLAG_ANYBODY still.
</nitpick>
> void
> +sysctlgatherstruct(addr, size, vecarr)
> + void *addr;
> + size_t size;
> + struct scgs_vector *vecarr;
> +{
> + struct scgs_vector *xp;
> + int tsiz, rv;
tsiz must be a size_t. I made the same error in one of my earlier
setgid removal patches and broke things on the alpha due to passing it
as a pointer to sysctlbyname() ;)
> +
> + for (xp = vecarr; xp->sysctl != NULL; xp++) {
> + assert(xp->offset <= size);
> + tsiz = xp->size;
> + rv = sysctlbyname(xp->sysctl, addr + xp->offset, &tsiz,
> + NULL, 0);
Strict CFLAGS settings will complain about void pointer
arithmetic. Granted, this is nitpicky, because with gcc it will work
right.
> +void
> +kget(idx, addr, size)
> + int idx;
> + void *addr;
> + size_t size;
> +{
> + char *symn; /* symbol name */
> + int rv, tsiz;
Again, tsiz should be size_t.
There will be some warnings about format arguments when using err with
%d and a size_t alpha. I'm not entirely sure to what type to cast to
best (my guess would be unsinged long, but you'd probably better ask
bde on this). There are some occurances of this around in src/ I
think, some produced by me, I'm afraid (will fix those soon ;).
- thomas
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?20010524214918.A2640>