Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Apr 2001 23:28:46 +0000 (GMT)
From:      Terry Lambert <tlambert@primenet.com>
To:        current@freebsd.org
Subject:   More on Bad Bug
Message-ID:  <200104172328.QAA01367@usr09.primenet.com>

next in thread | raw e-mail | index | archive | help
NB: To keep me in the loop, keep me in the Cc: list, since I
rarely follow -current these days).

I've further localized the bug to the freeing of credentials
associated with socket buffers, and being unrelated to the
crhold()/crfree() calls in socreate() and sodealloc().

Specifically, the bug exhibits as INVARIANTS complaining about
a 72 byte malloc -- it seems that in avoiding system call
structures, I neglected struct rusage -- which is exactly 72
bytes in size, as well.

There seems to be some bad code in soo_close(), which looks like:

	int
	soo_close(fp, p)
		struct file *fp;
		struct proc *p;   
	{
		int error = 0;

		fp->f_ops = &badfileops;
		if (fp->f_data)
			error = soclose((struct socket *)fp->f_data);
		fp->f_data = 0; 
		return (error);
	}

It seems to me this should be?

	int
	soo_close(fp, p)
		struct file *fp;
		struct proc *p;   
	{
		int error = 0;

		if (fp->f_data)
			error = soclose((struct socket *)fp->f_data);
		if (!error) {
			fp->f_data = 0; 
			fp->f_ops = &badfileops;
		}
		return (error);
	}

But it's not clear that this is correct for the socket code.

The INVARIANTS code is actually part of the problem here, since the
reference count on the credential is one of the fields stomped by
the WIERD_ADDR "to be safer".

This doesn't evidence as a problem with a double free in the case
that INVARIANTS aren't used, in that the decremented count in the
crfree() (which decrements prior to examining the value to see if
it is exactly zero), would continue to be non-zero, and not decrement
back to zero (resulting in the double free).

Credentials are actually used _AMAZINGLY_ much; it seems that they
are a good candidate for some optimization to throw away references
that aren't really necessary (for example, it seems to me that a
socket can not exist without an fdp referencing it, and the fdp has
a reference count on the cred which the socket inherits from the fdp,
so the fdp's reference protects the sockets reference, and so the
socket's reference doesn't really need to be reference counted).

In any case, I'm leaving in the panic patch I sent earlier, and am
now rebuilding with my ucred reference count moved past the area
stomped by INVARIANTS.  This should permit the INVARIANTS to catch
double frees (which they can't, otherwise, because of the refcnt
decrement making it look like the block was used after being freed,
and tricking INVARIANTS in free() in kern_malloc.c.

I guess no one else is interested in this bug hunt, or no one else
is using 30,000 sockets on any of their machines?


					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200104172328.QAA01367>