> On 11 Sep 2021, at 21:51, Alexander Bluhm <[email protected]> wrote:
>
> On Sat, Sep 11, 2021 at 04:05:31AM +0300, Vitaliy Makkoveev wrote:
>> On Fri, Sep 10, 2021 at 07:40:58PM +0200, Alexander Bluhm wrote:
>>> Hi,
>>>
>>> After converting syslogd messages to iov, UDP can use sendmsg(2)
>>> instead of snprintf().
>>>
>>> ok?
>>>
>>
>> I have one question below, but the diff is ok by mvs@ if it is not
>> significant.
>>
>> It is expected we could drop iov[6].iov_len bytes more from `msg' but
>> keep newline stuff when message size exceeds MAX_UDPMSG? The original
>> code has the different behaviour.
>
> There is no new line in UDP messages. In the UDP case iov[6].iov_len
> is 0. This is the default case I have added in the previous commit.
>
Already seen, thanks!
> iov[5] contains the message, and only this will be truncated. The
> redundant checks are there to see that no integer overflow is
> possible.
>
martijn@ worries someone could decrease MAX_UDPMSG definition to be
less then iov[0..4].iov_len max possible sum. This is not related to
your diff but I propose to add commentary with length limitations
near MAX_UDPMSG definition.
> bluhm
>
>>> Index: usr.sbin/syslogd/syslogd.c
>>> ===================================================================
>>> RCS file: /cvs/src/usr.sbin/syslogd/syslogd.c,v
>>> retrieving revision 1.269
>>> diff -u -p -r1.269 syslogd.c
>>> --- usr.sbin/syslogd/syslogd.c 10 Sep 2021 15:18:36 -0000 1.269
>>> +++ usr.sbin/syslogd/syslogd.c 10 Sep 2021 17:36:45 -0000
>>> @@ -1897,6 +1897,7 @@ void
>>> fprintlog(struct filed *f, int flags, char *msg)
>>> {
>>> struct iovec iov[IOVCNT], *v;
>>> + struct msghdr msghdr;
>>> int l, retryonce;
>>> char line[LOG_MAXLINE + 1], pribuf[13], greetings[500], repbuf[80];
>>> char ebuf[ERRBUFSIZE];
>>> @@ -2037,20 +2038,22 @@ fprintlog(struct filed *f, int flags, ch
>>>
>>> case F_FORWUDP:
>>> log_debug(" %s", f->f_un.f_forw.f_loghost);
>>> - l = snprintf(line, MINIMUM(MAX_UDPMSG + 1, sizeof(line)),
>>> - "%s%s%s%s%s%s%s", (char *)iov[0].iov_base,
>>> - (char *)iov[1].iov_base, (char *)iov[2].iov_base,
>>> - (char *)iov[3].iov_base, (char *)iov[4].iov_base,
>>> - (char *)iov[5].iov_base, (char *)iov[6].iov_base);
>>> - if (l < 0)
>>> - l = strlcpy(line, iov[5].iov_base, sizeof(line));
>>> - if (l >= sizeof(line))
>>> - l = sizeof(line) - 1;
>>> - if (l >= MAX_UDPMSG + 1)
>>> - l = MAX_UDPMSG;
>>> - if (sendto(f->f_file, line, l, 0,
>>> - (struct sockaddr *)&f->f_un.f_forw.f_addr,
>>> - f->f_un.f_forw.f_addr.ss_len) != l) {
>>> + l = iov[0].iov_len + iov[1].iov_len + iov[2].iov_len +
>>> + iov[3].iov_len + iov[4].iov_len + iov[5].iov_len +
>>> + iov[6].iov_len;
>>> + if (l > MAX_UDPMSG) {
>>> + l -= MAX_UDPMSG;
>>> + if (iov[5].iov_len > l)
>>> + iov[5].iov_len -= l;
>>> + else
>>> + iov[5].iov_len = 0;
>>> + }
>>> + memset(&msghdr, 0, sizeof(msghdr));
>>> + msghdr.msg_name = &f->f_un.f_forw.f_addr;
>>> + msghdr.msg_namelen = f->f_un.f_forw.f_addr.ss_len;
>>> + msghdr.msg_iov = iov;
>>> + msghdr.msg_iovlen = IOVCNT;
>>> + if (sendmsg(f->f_file, &msghdr, 0) == -1) {
>>> switch (errno) {
>>> case EADDRNOTAVAIL:
>>> case EHOSTDOWN:
>>> @@ -2063,7 +2066,7 @@ fprintlog(struct filed *f, int flags, ch
>>> break;
>>> default:
>>> f->f_type = F_UNUSED;
>>> - log_warn("sendto \"%s\"",
>>> + log_warn("sendmsg to \"%s\"",
>>> f->f_un.f_forw.f_loghost);
>>> break;
>>> }