sflow / host-sflow

host-sflow agent
http://sflow.net
Other
146 stars 55 forks source link

Broken line parsing in readInterfaces #32

Open carlanton opened 5 years ago

carlanton commented 5 years ago

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

sflow commented 5 years ago

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.

carlanton commented 5 years ago

Yes, that workaround fixes the problem!

sflow commented 5 years ago

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.