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

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

Details

Reviewers

markj
dmitry.wagin_ya.ru

Commits

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

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

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 43063
Build 39951: arc lint + arc unit

Event Timeline

asomers created this revision.Nov 30 2021, 5:56 PM

Herald added a subscriber: imp. · View Herald TranscriptNov 30 2021, 5:56 PM

asomers requested review of this revision.Nov 30 2021, 5:56 PM

Harbormaster completed remote builds in B43063: Diff 99253.Nov 30 2021, 5:56 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.

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

In D33199#750243, @markj wrote:

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.

In D33199#750243, @markj wrote:

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.

Closed by commit rG943c446629e3: Revert "libc: Some enhancements to syslog(3)" (authored by asomers). · Explain WhyNov 30 2021, 6:12 PM

This revision was automatically updated to reflect the committed changes.

asomers added a commit: rG943c446629e3: Revert "libc: Some enhancements to syslog(3)".

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

In D33199#750268, @dmitry.wagin_ya.ru wrote:

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...

In D33199#750825, @markj wrote:

In D33199#750268, @dmitry.wagin_ya.ru wrote:

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).

In D33199#750847, @dmitry.wagin_ya.ru wrote:

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"?

In D33199#750847, @dmitry.wagin_ya.ru wrote:

In D33199#750825, @markj wrote:

In D33199#750268, @dmitry.wagin_ya.ru wrote:

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?

In D33199#750851, @asomers wrote:

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.

In D33199#750855, @markj wrote:

In D33199#750851, @asomers wrote:

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.

In D33199#750851, @asomers wrote:

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.

In D33199#750852, @markj wrote:

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 D33199#750858, @dmitry.wagin_ya.ru wrote:

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

The original fix only exposed the infinite loop issue.

Revision Contents
Changeset List

Path

Size

lib/

libc/

gen/

syslog.c

15 lines

Diff 99253

View Options

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