stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.29k stars 1.71k forks source link

libmodbus uses select() instead of poll() #583

Open rakshitaSiemens opened 3 years ago

rakshitaSiemens commented 3 years ago

libmodbus version

3.1.4-2

OS and/or distribution

Debian Linux AMD 32-bit

Environment

32 bit(AMD)

Description

We are using 3.1.4-2 version of libmodbus. We are facing “buffer overflow error and core dump shows below output-

6 0xb6fce11e in __GI___fortify_fail (msg=0xb7044182 "buffer overflow detected") at fortify_fail.c:44

7 0xb6fcc559 in __GI___chk_fail () at chk_fail.c:28

8 0xb6fcdffa in __fdelt_chk (d=1138) at fdelt_chk.c:25

9 0xb7ef4a72 in ?? () from /lib/i386-linux-gnu/libmodbus.so.5

10 0xb7ef4d82 in ?? () from /lib/i386-linux-gnu/libmodbus.so.5

11 0x006077f5 in WAGO::connectandread() ()

12 0x0060a3f1 in WAGO::WAGO(WagoControllerConfiguration, QString, int, DsuDiagnostics*) ()

13 0x00545955 in WagoThread::run() ()

14 0xb7417666 in ?? () from /lib/i386-linux-gnu/sse2/libQt5Core.so.5

15 0xb734afd2 in start_thread (arg=) at pthread_create.c:486

16 0xb6fbd6d6 in clone () at ../sysdeps/unix/sysv/linux/i386/clone.S:108

With this looks like libmodbus is using select() and not poll() due to which when FD count is more than 1024 application is terminated. How can we resolve this issue? We cant reduce FD count as we are using some 3rd party component which opens many FDs for Database operations. Is there a work around for this?

Expected behaviour

libmodbus should use poll()

Actual behaviour

libmodbus uses select()

karlp commented 3 years ago

Duplicate of https://github.com/stephane/libmodbus/issues/373 but this time with a little more context. (select is used because is available on windows, poll is not)

rakshitaSiemens commented 3 years ago

Is there a way to get a version which uses poll() for Linux? As we are using linux package of libmodbus

rakshitaSiemens commented 3 years ago

Duplicate of #373 but this time with a little more context. (select is used because is available on windows, poll is not) Is there a way to get a version which uses poll() for Linux? As we are using linux package of libmodbus

iostapyshyn commented 3 years ago

I would like to address this. The library itself calls to select() on 4 occasions, each time checking only one single file descriptor, and modbus-rtu.c already uses a replacement function win32_ser_select() on Windows. Would providing a similar drop-in replacement function for TCP on Windows and using poll() on other operating systems be a good idea?

Another issue is the different timeout argument: poll() expects a value in milliseconds, but struct timeval is used throughout the library and in the API. win32_ser_select() uses

int msec = tv->tv_sec * 1000 + tv->tv_usec / 1000;

Would that be an adequate solution?

Please correct me if I'm missing something.

rakshitaSiemens commented 3 years ago

i think it would be. So replacing select() with poll() is what we are looking for. Also regarding timeout, i believe its internal implementation and should not impact the poll() functionality right?

rakshitaSiemens commented 3 years ago

Hi Team, Do you have any update on this issue? When can we expect a package with the fix?

fcntlcc commented 2 years ago

I have the same problem. And I make a patch and test it under Debian. You are welcom to test it for other systems. My patch is at: https://github.com/fcntlcc/libmodbus/commit/244006169fe6e4501f2d4fea9b4b0f04a5927f06

modem-man-gmx commented 1 year ago

Why the hell poll()? If the code needs to make a leap and to split in legacy vs. modern API, we could talk about epoll(). check this benchmark at libevent

alongL commented 1 year ago

I don't think libmodbus should support poll.
Modbus is not some internet service protocol.
It's used mainly on some machines.
No need to support many clients more than 1024.

due to which when FD count is more than 1024 

This may be miss using of libmodbus.

stefandxm commented 10 months ago

Any news about this? Was about to create a patch for this myself, but I can see now that its already fixed just merged.

Flow86 commented 7 months ago

This may be miss using of libmodbus.

you could have applications using a big amount of file descriptors in the background, and still using libmodbus for only a part of it.

I recently also got affected by this issue. Is it merged into upstream?

Flow86 commented 7 months ago

I adapted the patch from @fcntlcc (thank you) and changed all other places which were affected.

Feel free to use my commit (no warranties though ...)