Dear Maintainer
I have identified a bug in the lb app, that causes it to write beyond the tail, into kernel owned part of the ring.
it was introduced in 0bc24ddd04b0070a8ef8a23dc4dfacb45d3f2a9a where lim was changed from type uint32_t to int64_t and replaced the call to nm_ring_space with manual computation.
The issue arises in the line
|
mf = (struct morefrag *)ring->sem; |
|
lim = ring->tail - mf->shadow_head; |
because head and tail are uint32_t the result of the subtraction will wraparound to near INT_MAX. if lim was int32_t this would implicitly cast to the correct negative number, but since it is 64 bit, the large positive number fits and it is not converted, leaving lim as a huge positive number.
This causes the for loop to dump all packets on the overflow queue onto the ring, potentially overwriting the ring multiple times over.
The fix is simply to declare lim to be int32_t, but I am wondering whether it is advisable to rely on overflow+implicit cast in general.
Regards
Emil Lynge
Dear Maintainer
I have identified a bug in the
lbapp, that causes it to write beyond the tail, into kernel owned part of the ring.it was introduced in 0bc24ddd04b0070a8ef8a23dc4dfacb45d3f2a9a where
limwas changed from typeuint32_ttoint64_tand replaced the call tonm_ring_spacewith manual computation.The issue arises in the line
netmap/apps/lb/lb.c
Lines 982 to 983 in 609ba10
because head and tail are
uint32_tthe result of the subtraction will wraparound to near INT_MAX. iflimwasint32_tthis would implicitly cast to the correct negative number, but since it is 64 bit, the large positive number fits and it is not converted, leavinglimas a huge positive number.This causes the for loop to dump all packets on the overflow queue onto the ring, potentially overwriting the ring multiple times over.
The fix is simply to declare
limto beint32_t, but I am wondering whether it is advisable to rely on overflow+implicit cast in general.Regards
Emil Lynge