Page MenuHomeFreeBSD

Revert "libc: Some enhancements to syslog(3)"
ClosedPublic

Authored by asomers on Nov 30 2021, 5:56 PM.

Details

Summary

This reverts commit 2886c93d1bca231260ebc01d4205743ca781f3c7.
The original commit has two problems:

  • It sets SO_SNDBUF to be as large as MAXLINE. But for unix domain sockets, the send buffer is bypassed. Packets go directly to the peer's receive buffer, so setting and querying SO_SNDBUF is ineffective. To ensure that the socket can accept messages of a certain size, it would be necessary to add a SO_PEERRCVBUF socket option that could query the connected peer's receive buffer size.
  • It sets MAXLINE to 8 kB, which is larger than the default sockbuf size of 4 kB. That's ok for the builtin syslogd, which sets its recvbuf to 80 kB, but not ok for alternative sysloggers, like rsyslogd, which use the default size.

As a consequence, writing messages of more than 4 kB with syslog() as a
non-root user while running rsyslogd would cause the logging application
to spin indefinitely within syslog().

PR: 260126
MFC: 2 weeks
Sponsored by: Axcient

Test Plan

Manually tested as described in the PR

Diff Detail

Repository
rG FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Ok with me since it fixes a regression. I do wonder if it's finally time to up the default socket buffer sizes though. I'm a little surprised that rsyslog uses the default.

This revision is now accepted and ready to land.Nov 30 2021, 6:03 PM

Ok with me since it fixes a regression. I do wonder if it's finally time to up the default socket buffer sizes though. I'm a little surprised that rsyslog uses the default.

Many modern programs generate large log messages, reducing the max message size to 1kb is a bad idea. IMHO, this is regression.

Ok with me since it fixes a regression. I do wonder if it's finally time to up the default socket buffer sizes though. I'm a little surprised that rsyslog uses the default.

Yeah, there are several possible solutions to the original problem. Raise the default socket buffer sizes, add SO_RCVPEERBUF, make retries time-limited, and probably others too. I don't think any is obviously the best option.

Set SO_SNDBUF change what limit net.local.dgram.maxdgram=2048

Set SO_SNDBUF change what limit net.local.dgram.maxdgram=2048

I don't understand what you mean by this. As Alan pointed out, the value of SO_SNDBUF is not very important, it just controls the maximum size of a datagram. When a message is sent over a unix socket, the message is placed directly in the receive buffer. So, we need some other solution to enable large syslog messages.

I'm not sure about the SO_RCVPEERBUF idea: the receiver can change its recv buffer size at any time, so there's still some potential for problems. The default socket buffer sizes have been the same for a very long time, and I suspect that we can increase them without harm...

Set SO_SNDBUF change what limit net.local.dgram.maxdgram=2048

I don't understand what you mean by this. As Alan pointed out, the value of SO_SNDBUF is not very important, it just controls the maximum size of a datagram. When a message is sent over a unix socket, the message is placed directly in the receive buffer. So, we need some other solution to enable large syslog messages.

I'm not sure about the SO_RCVPEERBUF idea: the receiver can change its recv buffer size at any time, so there's still some potential for problems. The default socket buffer sizes have been the same for a very long time, and I suspect that we can increase them without harm...

I fully understand what Alan wrote. But the SO_SNDBUF setting is also needed. It doesn't matter what size the buffer is on reception (syslogd 80kb), if you send messages larger than SO_SNDBUF (by default from net.local.dgram.maxdgram) nothing will work.

Real problem is not solved (infinite loop on sending).

The feature which is needed for real users is destroying in the RELEASE branch (log messages over 1kb).

I fully understand what Alan wrote. But the SO_SNDBUF setting is also needed. It doesn't matter what size the buffer is on reception (syslogd 80kb), if you send messages larger than SO_SNDBUF (by default from net.local.dgram.maxdgram) nothing will work.

But syslog(3) already handled that problem by splitting long messages up into multiple messages of no more than MAXLINE characters each. If I try to log a very long message, it still gets logged, but as multiple separate messages. That doesn't require increasing MAXLINE.

Real problem is not solved (infinite loop on sending).

The feature which is needed for real users is destroying in the RELEASE branch (log messages over 1kb).

What do you mean by "destroying in the RELEASE branch"?

Set SO_SNDBUF change what limit net.local.dgram.maxdgram=2048

I don't understand what you mean by this. As Alan pointed out, the value of SO_SNDBUF is not very important, it just controls the maximum size of a datagram. When a message is sent over a unix socket, the message is placed directly in the receive buffer. So, we need some other solution to enable large syslog messages.

I'm not sure about the SO_RCVPEERBUF idea: the receiver can change its recv buffer size at any time, so there's still some potential for problems. The default socket buffer sizes have been the same for a very long time, and I suspect that we can increase them without harm...

I fully understand what Alan wrote. But the SO_SNDBUF setting is also needed. It doesn't matter what size the buffer is on reception (syslogd 80kb), if you send messages larger than SO_SNDBUF (by default from net.local.dgram.maxdgram) nothing will work.

Yes, it's necessary to adjust SO_SNDBUF, but not sufficient. It does matter what size the buffer is on reception, libc should not assume that syslogd is the only possible receiver.

Real problem is not solved (infinite loop on sending).

The feature which is needed for real users is destroying in the RELEASE branch (log messages over 1kb).

Yes, and the first attempt at fixing this needs some more work. I would like to help make it happen. So, how shall we fix it?

But syslog(3) already handled that problem by splitting long messages up into multiple messages of no more than MAXLINE characters each. If I try to log a very long message, it still gets logged, but as multiple separate messages. That doesn't require increasing MAXLINE.

No, syslog(3) will truncate long messages. If you're using logger(1) to test, note that it handles splitting itself.

But syslog(3) already handled that problem by splitting long messages up into multiple messages of no more than MAXLINE characters each. If I try to log a very long message, it still gets logged, but as multiple separate messages. That doesn't require increasing MAXLINE.

No, syslog(3) will truncate long messages. If you're using logger(1) to test, note that it handles splitting itself.

Ahh, that's it. I was using daemon(8) to test, and it does its own splitting.

Real problem is not solved (infinite loop on sending).

The feature which is needed for real users is destroying in the RELEASE branch (log messages over 1kb).

What do you mean by "destroying in the RELEASE branch"?

Committed to main. I'll MFC to stable/13 in two weeks. But the offending commit was never MFCed to stable/12, so there's no need to MFC there.

Yes, it's necessary to adjust SO_SNDBUF, but not sufficient. It does matter what size the buffer is on reception, libc should not assume that syslogd is the only possible receiver.

Of course, syslogd is not the only possible receiver. But there is no way for the sender to influence the receiver. syslog(3) should just not hang.

Real problem is not solved (infinite loop on sending).

The feature which is needed for real users is destroying in the RELEASE branch (log messages over 1kb).

Yes, and the first attempt at fixing this needs some more work. I would like to help make it happen. So, how shall we fix it?

In my opinion it is necessary to limit the number of attempts.

In my opinion, the original fix is indirectly related to the problem described by Alan.

In my opinion, the original fix is indirectly related to the problem described by Alan.

The original fix only exposed the infinite loop issue.