[systemd-devel] [PATCH] libudev: fix check for too long packet
Don't use recvmsg(2) return value to check for too long packets (it doesn't work) but MSG_TRUNC flag. --- src/libudev/libudev-monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c index 484fefe..d8e551b 100644 --- a/src/libudev/libudev-monitor.c +++ b/src/libudev/libudev-monitor.c @@ -609,7 +609,7 @@ retry: return NULL; } -if (buflen < 32 || (size_t)buflen >= sizeof(buf)) { +if (buflen < 32 || smsg.msg_flags & MSG_TRUNC) { log_debug("invalid message length"); return NULL; } -- 2.1.4 ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
Hi On Sun, Jan 18, 2015 at 10:57 PM, Topi Miettinen wrote: > Don't use recvmsg(2) return value to check for too long packets > (it doesn't work) but MSG_TRUNC flag. > --- > src/libudev/libudev-monitor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Applied! Thanks David > diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c > index 484fefe..d8e551b 100644 > --- a/src/libudev/libudev-monitor.c > +++ b/src/libudev/libudev-monitor.c > @@ -609,7 +609,7 @@ retry: > return NULL; > } > > -if (buflen < 32 || (size_t)buflen >= sizeof(buf)) { > +if (buflen < 32 || smsg.msg_flags & MSG_TRUNC) { > log_debug("invalid message length"); > return NULL; > } > -- > 2.1.4 > > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
On Sun, 18.01.15 23:57, Topi Miettinen (toiwo...@gmail.com) wrote: > Don't use recvmsg(2) return value to check for too long packets > (it doesn't work) but MSG_TRUNC flag. Why precisely doesn't this work? I mean, it will consider messages that are exactly as large as the buffer as too long, but otherwise the old check should be fine, no? (The new check is much nicer though, admittedly) > --- > src/libudev/libudev-monitor.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c > index 484fefe..d8e551b 100644 > --- a/src/libudev/libudev-monitor.c > +++ b/src/libudev/libudev-monitor.c > @@ -609,7 +609,7 @@ retry: > return NULL; > } > > -if (buflen < 32 || (size_t)buflen >= sizeof(buf)) { > +if (buflen < 32 || smsg.msg_flags & MSG_TRUNC) { > log_debug("invalid message length"); > return NULL; > } > -- > 2.1.4 > > ___ > systemd-devel mailing list > systemd-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/systemd-devel Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
On 01/23/15 03:06, Lennart Poettering wrote: > On Sun, 18.01.15 23:57, Topi Miettinen (toiwo...@gmail.com) wrote: > >> Don't use recvmsg(2) return value to check for too long packets >> (it doesn't work) but MSG_TRUNC flag. > > Why precisely doesn't this work? I mean, it will consider messages > that are exactly as large as the buffer as too long, but otherwise the > old check should be fine, no? It doesn't work because the return value of recvmsg() never exceeds the buffer size, so too large packets are never detected. -Topi > > (The new check is much nicer though, admittedly) > >> --- >> src/libudev/libudev-monitor.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c >> index 484fefe..d8e551b 100644 >> --- a/src/libudev/libudev-monitor.c >> +++ b/src/libudev/libudev-monitor.c >> @@ -609,7 +609,7 @@ retry: >> return NULL; >> } >> >> -if (buflen < 32 || (size_t)buflen >= sizeof(buf)) { >> +if (buflen < 32 || smsg.msg_flags & MSG_TRUNC) { >> log_debug("invalid message length"); >> return NULL; >> } >> -- >> 2.1.4 >> >> ___ >> systemd-devel mailing list >> systemd-devel@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/systemd-devel > > > Lennart > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
On Fri, Jan 23, 2015 at 05:29:46PM +, Topi Miettinen wrote: > On 01/23/15 03:06, Lennart Poettering wrote: > > On Sun, 18.01.15 23:57, Topi Miettinen (toiwo...@gmail.com) wrote: > > > >> Don't use recvmsg(2) return value to check for too long packets > >> (it doesn't work) but MSG_TRUNC flag. > > > > Why precisely doesn't this work? I mean, it will consider messages > > that are exactly as large as the buffer as too long, but otherwise the > > old check should be fine, no? > > It doesn't work because the return value of recvmsg() never exceeds the > buffer size, so too large packets are never detected. It doesn't have to exceed the buffer size, it just has to equal it. So packets of size sizeof(buf) and packets greater than that would be detected as overlong. So the check wasn't wrong, just inefficient-by-one. > > (The new check is much nicer though, admittedly) Agreed. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
On Fri, 23.01.15 17:29, Topi Miettinen (toiwo...@gmail.com) wrote: > On 01/23/15 03:06, Lennart Poettering wrote: > > On Sun, 18.01.15 23:57, Topi Miettinen (toiwo...@gmail.com) wrote: > > > >> Don't use recvmsg(2) return value to check for too long packets > >> (it doesn't work) but MSG_TRUNC flag. > > > > Why precisely doesn't this work? I mean, it will consider messages > > that are exactly as large as the buffer as too long, but otherwise the > > old check should be fine, no? > > It doesn't work because the return value of recvmsg() never exceeds the > buffer size, so too large packets are never detected. But the test was ">=", not ">". So the old code *did* recognize all too large packets, though it would already do so one byte earlier than your new check... Lennart -- Lennart Poettering, Red Hat ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] libudev: fix check for too long packet
On 01/23/15 17:43, Lennart Poettering wrote: > On Fri, 23.01.15 17:29, Topi Miettinen (toiwo...@gmail.com) wrote: > >> On 01/23/15 03:06, Lennart Poettering wrote: >>> On Sun, 18.01.15 23:57, Topi Miettinen (toiwo...@gmail.com) wrote: >>> Don't use recvmsg(2) return value to check for too long packets (it doesn't work) but MSG_TRUNC flag. >>> >>> Why precisely doesn't this work? I mean, it will consider messages >>> that are exactly as large as the buffer as too long, but otherwise the >>> old check should be fine, no? >> >> It doesn't work because the return value of recvmsg() never exceeds the >> buffer size, so too large packets are never detected. > > But the test was ">=", not ">". So the old code *did* recognize all > too large packets, though it would already do so one byte earlier than > your new check... True. What should be considered too large, a full buffer (which might not contain a trailing zero, so the strcmp later could fall off of the buffer...), or buffer size - 1 (the last byte is not explicitly set to zero, so badness could happen anyway)? -Topi > > Lennart > ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel