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.

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
Changes from all commits
Commits
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
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

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)
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<sockaddr*>(&saddr), sizeof(saddr)) < 0)
{
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