Bug 17583
| Summary: | NETATALK code can corrupt mbuf free lists | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | iedowse <iedowse> | ||||
| Component: | kern | Assignee: | iedowse | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Only Me | ||||||
| Priority: | Normal | ||||||
| Version: | 3.4-STABLE | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Responsible Changed From-To: freebsd-bugs->julian Julian, could you check this one out? Ian submits quality PR's. :-) Responsible Changed From-To: julian-> iedowse Ian Dowse is now a committer. Ian, I think you'll know best whether or not this was fixed and should be able to easily close the PR if it was. State Changed From-To: open->closed My analysis in the PR wasn't correct; Julian found that the code path I suggested couldn't occur. However the patch did cure the panics, so the real problem is possibly a more complex situation. The machine in question is no longer in service and the patch has been committed and MFC'd, so there's no point in keeping the PR open any longer. |
In certain circumstances, the NETATALK code can cause an mbuf chain to be freed multiple times. The code uses a 'struct aarptab' to store information about appletalk addresses, which includes an mbuf pointer 'aap_hold'. In most places, the code is careful about ensuring that mbuf chains referenced by this mechanism are freed only once, but there is a subtle problem in at_aarpinput(). The code fragment looks something like: if (aat->aat_hold != NULL) { (*ac->ac_if.if_output)(&ac->ac_if, aat->aat_hold, ...); aat->aat_hold = NULL; } The problem here is that when if_output() is called, aat->aat_hold contains a non-NULL pointer. If the interface if_output routine calls aarpresolve(), then aarpresolve() may free aat->aat_hold. This can result in aat_hold being m_freem'd twice, since if_output will also free it. The simple solution is to ensure that aat_hold is NULLed out before calling the if_output routine. The patch below does this by copying the mbuf pointer into a local variable so that aat->aat_hold can be NULL when if_output is called. How-To-Repeat: Connecting a FreeBSD router to a busy network seemed to trigger this problem every few hours, but it should be easy to construct a sequence of packet arrivals which cause it directly.