Project

General

Profile

Actions

Bug #4801

closed

af-packet: tpacket v3 socket reference handling broken

Added by Victor Julien 11 months ago. Updated 11 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

The v3 code references the socket AFPParsePacketV3 using AFPRefSocket, so per packet.

However, in contrast to the V2 code, the release callback AFPReleasePacketV3 does not release the reference.

I've confirmed that this reference counter only increases, and most likely wraps around regularly in high speed networks as the counter is only an int (side note: wrap arounds of signed ints are undefined behavior).

Its easy to add the deref logic of course, but the question is: do we really need it? In V3 we seem to have never have had it, so can we keep doing without it?


Related issues 1 (0 open1 closed)

Related to Bug #4804: af-packet: tpacket v3 if/down logic brokenClosedVictor JulienActions
Actions #1

Updated by Victor Julien 11 months ago

  • Related to Bug #4804: af-packet: tpacket v3 if/down logic broken added
Actions #2

Updated by Eric Leblond 11 months ago

The reference counting system has been introduced in:

commit 13f13b6d7ebe6999e6c8e43c72a3055375cd6556
Author: Eric Leblond <>
Date: Mon Sep 3 16:43:45 2012 +0200

af-packet: rework socket transition phase.
Suricata was not able to start cleanly in AF_PACKET with default
suricata.yaml file if there was no eth1 on the system. This patch
fixes this issue and rework the socket transition phase to fix
some serious issues (file descriptor leak) found when fixing this
problem.
Every 20 seconds it displays a message to the user to warn him about
the interface not being accessible:
[ERRCODE: SC_ERR_AFP_CREATE(196)] - Can not open iface 'eth1'

It is just used to avoid closing a socket that could still have to be used in IPS mode by other packet that need to be forwarded. Given the fact it is just about fixing a transition and that the iface the socket is attached to is probably down, it would be simpler to just set the socket to -1 (atomically I think) and handle the error correctly instead of doing reference counting.

Actions #3

Updated by Victor Julien 11 months ago

  • Status changed from Feedback to Closed
  • Assignee changed from Eric Leblond to Victor Julien
  • Target version set to 7.0rc1
Actions

Also available in: Atom PDF