host-sflow icon indicating copy to clipboard operation
host-sflow copied to clipboard

Broken line parsing in readInterfaces

Open carlanton opened this issue 6 years ago • 3 comments

Hi,

The parsing of /proc/net/dev is broken if the lines are longer than MAX_PROC_LINE_CHARS. There is a comment that says that fgets will chop the line off if it's longer, however the next call to fgets will return the rest of the line.

Relevant code: https://github.com/sflow/host-sflow/blob/master/src/Linux/readInterfaces.c#L653-L673

My /proc/net/dev looks something like this:

Inter-|   Receive                                                |  Transmit
 face |bytes    packets errs drop fifo frame compressed multicast|bytes    packets errs drop fifo colls carrier compressed
            eth0: 359401051 4518683    0    0    0     0          0    513091 144790237  522864    0    0    0     0       0          0
bond-interface-a: 286042294609005 1770579283801    0 180681106    0     0          0  20252445 23210149855391943 15484818667769    0  213    0     0       0          0
bond-interface-b: 129287069135225 1719490381267    5 2143401    0     0          0  14099075 20773782419083321 13758778336373    0 10414    0     0       0          0

Since the lines for bond-interface-a and bond-interface-b are longer than MAX_PROC_LINE_CHARS, the parser thinks that the interface counters are device names and then tries to call ioctl SIOCGIFFLAGS on the device 0. This generates a lot of errors :)

I'm not sure what would be the best way to fix this. Should parseNextTok return NULL if it can't find the separator? Or should the while-loop make sure that we skip trailing line data?

The same read pattern seems to be used on multiple places in the source code.

Thanks, Anton

carlanton avatar May 24 '19 07:05 carlanton

Looks like a misunderstanding of how fgets works. The simplest workaround is to increase MAX_PROC_LINE_CHARS. Please confirm that it works when you do that.

I'll look for the best way to detect and discard the rest of the line if we get a freakishly long one.

sflow avatar May 24 '19 16:05 sflow

Yes, that workaround fixes the problem!

carlanton avatar May 25 '19 14:05 carlanton

I checked in a more resilient fix, in the form of a new util function called my_readline() to replace the use of fgets(). If you get a chance, please 'git pull' and test.

sflow avatar May 28 '19 23:05 sflow