Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bind() instead of setsockopt() for binding raw sockets on Linux #1646

Merged
merged 4 commits into from
Dec 3, 2024
Merged

Use bind() instead of setsockopt() for binding raw sockets on Linux #1646

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc762e8
Update to `bind()`
seladb Nov 27, 2024
67f745d
Merge branch 'dev' into use-bind-in-raw-sockets2
seladb Dec 2, 2024
c43e110
Remove the `struct` keyword
seladb Dec 2, 2024
e32c6ce
Merge branch 'dev' into use-bind-in-raw-sockets2
egecetin Dec 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions Pcap++/src/RawSocketDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ namespace pcpp
int fd = socket(AF_PACKET, SOCK_RAW, htobe16(ETH_P_ALL));
if (fd < 0)
{
PCPP_LOG_ERROR("Failed to create raw socket. Error code was " << errno);
PCPP_LOG_ERROR("Failed to create raw socket. Error code was " << strerror(errno));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not checked the full code so maybe not important but strerror is not thread safe

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess errno isn't thread safe either, so we should handle it differently if we want to be fully thread safe. In this case it might not be required beacuse it's the creation and binding of the socket...

Copy link
Collaborator

@egecetin egecetin Dec 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually errno is thread safe since it is thread local but there are other strerror functions in library so better to fix it all later

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, I think you're right. I'll open a separate PR for it

return false;
}

Expand Down Expand Up @@ -505,12 +505,15 @@ namespace pcpp
}

// bind raw socket to interface
struct ifreq ifr;
memset(&ifr, 0, sizeof(ifr));
snprintf(ifr.ifr_name, sizeof(ifr.ifr_name), "%s", ifaceName.c_str());
if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, (void*)&ifr, sizeof(ifr)) == -1)
struct sockaddr_ll saddr;
memset(&saddr, 0, sizeof(saddr));
saddr.sll_family = AF_PACKET;
saddr.sll_protocol = htons(ETH_P_ALL);
saddr.sll_ifindex = if_nametoindex(ifaceName.c_str());

if (bind(fd, reinterpret_cast<struct sockaddr*>(&saddr), sizeof(saddr)) < 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Do we need the struct keyword here and on L508?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c43e110

{
PCPP_LOG_ERROR("Cannot bind raw socket to interface '" << ifaceName << "'");
PCPP_LOG_ERROR("Cannot bind raw socket to interface '" << ifaceName << "': " << strerror(errno));
egecetin marked this conversation as resolved.
Show resolved Hide resolved
::close(fd);
return false;
}
Expand Down
Loading