[systemd-devel] [PATCH] libudev: fix check for too long packet

2015-01-18 Thread Topi Miettinen
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

2015-01-18 Thread David Herrmann
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

2015-01-22 Thread Lennart Poettering
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

2015-01-23 Thread Topi Miettinen
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

2015-01-23 Thread Zbigniew Jędrzejewski-Szmek
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

2015-01-23 Thread Lennart Poettering
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

2015-01-23 Thread Topi Miettinen
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