From: Nikolay Aleksandrov <niko...@cumulusnetworks.com> Date: Tue, 27 Oct 2015 14:31:48 +0100
> On 10/27/2015 01:12 PM, Roopa Prabhu wrote: >> From: Roopa Prabhu <ro...@cumulusnetworks.com> >> >> Problem Description: >> We can add fdbs pointing to the bridge with NULL ->dst but that has a >> few race conditions because br_fdb_insert() is used which first creates >> the fdb and then, after the fdb has been published/linked, sets >> "is_local" to 1 and in that time frame if a packet arrives for that fdb >> it may see it as non-local and either do a NULL ptr dereference in >> br_forward() or attach the fdb to the port where it arrived, and later >> br_fdb_insert() will make it local thus getting a wrong fdb entry. >> Call chain br_handle_frame_finish() -> br_forward(): >> But in br_handle_frame_finish() in order to call br_forward() the dst >> should not be local i.e. skb != NULL, whenever the dst is >> found to be local skb is set to NULL so we can't forward it, >> and here comes the problem since it's running only >> with RCU when forwarding packets it can see the entry before "is_local" >> is set to 1 and actually try to dereference NULL. >> The main issue is that if someone sends a packet to the switch while >> it's adding the entry which points to the bridge device, it may >> dereference NULL ptr. This is needed now after we can add fdbs >> pointing to the bridge. This poses a problem for >> br_fdb_update() as well, while someone's adding a bridge fdb, but >> before it has is_local == 1, it might get moved to a port if it comes >> as a source mac and then it may get its "is_local" set to 1 >> >> This patch changes fdb_create to take is_local and is_static as >> arguments to set these values in the fdb entry before it is added to the >> hash. Also adds null check for port in br_forward. >> >> Reported-by: Nikolay Aleksandrov <niko...@cumulusnetworks.com> >> Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com> >> --- >> v2 - fix compile error reported by kbuild test robot. >> Accidently i had posted an older version of the patch >> > > Thanks for fixing this, > > Fixes: 3741873b4f73 ("bridge: allow adding of fdb entries pointing to the > bridge device") > Reviewed-by: Nikolay Aleksandrov <niko...@cumulusnetworks.com> This still doesn't compile: net/bridge/br_fdb.c: In function ‘br_fdb_external_learn_add’: net/bridge/br_fdb.c:1103:3: error: too few arguments to function ‘fdb_create’ net/bridge/br_fdb.c:495:37: note: declared here