Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* invalid read in
@ 2020-02-03 13:54 dyslexicatheist
  2020-02-03 16:31 ` Phil Sutter
  0 siblings, 1 reply; 5+ messages in thread
From: dyslexicatheist @ 2020-02-03 13:54 UTC (permalink / raw)
  To: netfilter-devel

Hello,

I've written a filter to parse out punicode from DNS payloads and rewrite the packet in case it contains any IDN (xn--) marker unless the IDN is on a whitelist. Valgrind reports that nfq_create_queue() returns uninitialized
bytes resulting in thiserror:


sudo valgrind --tool=memcheck --leak-check=yes --show-reachable=yes \
           --num-callers=20 --track-fds=yes --track-origins=yes -s \
            ./nfq --syslog --facility LOG_LOCAL0 --log-level info \
                  --port 53  --renice -20 --rewrite-answer
      ==714384==
      ==714384== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
      ==714384==    at 0x4B977C7: sendto (sendto.c:27)
      ==714384==    by 0x486BE02: nfnl_send (in /usr/lib/x86_64-linux-gnu/libnfnetlink.so.0.2.0)
      ==714384==    by 0x486DBD2: nfnl_query (in /usr/lib/x86_64-linux-gnu/libnfnetlink.so.0.2.0)
      ==714384==    by 0x4A73995: nfq_set_mode (libnetfilter_queue.c:639)
      ==714384==    by 0x10B247: start_nfqueue_processing (nfq.c:532)
      ==714384==    by 0x10C289: main (nfq.c:987)
      ==714384==  Address 0x1ffefefbfd is on thread 1's stack
      ==714384==  in frame #3, created by nfq_set_mode (libnetfilter_queue.c:623)
      ==714384==  Uninitialised value was created by a stack allocation
      ==714384==    at 0x10A1B0: ??? (in /src/nfq/src/nfq)

After searching on this list archive, I found 1 question but without a follow-up answer:
https://marc.info/?l=netfilter-devel&m=137132916826745&w=4

Having already spent over a day chasing this. Not having come across other cases on github except this person self reporting[1] made me think it must be indeed something in my code that I'm missing and that could have triggered this. Or is it really rare (harmless) bug in libnetfilter?

[1] https://github.com/misje/dhcpoptinj/issues/5

thanks for any help,
~DA

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: invalid read in
  2020-02-03 13:54 invalid read in dyslexicatheist
@ 2020-02-03 16:31 ` Phil Sutter
  2020-02-03 17:14   ` dyslexicatheist
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2020-02-03 16:31 UTC (permalink / raw)
  To: dyslexicatheist; +Cc: netfilter-devel

Hi,

On Mon, Feb 03, 2020 at 01:54:31PM +0000, dyslexicatheist wrote:
> I've written a filter to parse out punicode from DNS payloads and rewrite the packet in case it contains any IDN (xn--) marker unless the IDN is on a whitelist. Valgrind reports that nfq_create_queue() returns uninitialized
> bytes resulting in thiserror:
> 
> 
> sudo valgrind --tool=memcheck --leak-check=yes --show-reachable=yes \
>            --num-callers=20 --track-fds=yes --track-origins=yes -s \
>             ./nfq --syslog --facility LOG_LOCAL0 --log-level info \
>                   --port 53  --renice -20 --rewrite-answer
>       ==714384==
>       ==714384== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
>       ==714384==    at 0x4B977C7: sendto (sendto.c:27)
>       ==714384==    by 0x486BE02: nfnl_send (in /usr/lib/x86_64-linux-gnu/libnfnetlink.so.0.2.0)
>       ==714384==    by 0x486DBD2: nfnl_query (in /usr/lib/x86_64-linux-gnu/libnfnetlink.so.0.2.0)
>       ==714384==    by 0x4A73995: nfq_set_mode (libnetfilter_queue.c:639)
>       ==714384==    by 0x10B247: start_nfqueue_processing (nfq.c:532)
>       ==714384==    by 0x10C289: main (nfq.c:987)
>       ==714384==  Address 0x1ffefefbfd is on thread 1's stack
>       ==714384==  in frame #3, created by nfq_set_mode (libnetfilter_queue.c:623)
>       ==714384==  Uninitialised value was created by a stack allocation
>       ==714384==    at 0x10A1B0: ??? (in /src/nfq/src/nfq)
> 
> After searching on this list archive, I found 1 question but without a follow-up answer:
> https://marc.info/?l=netfilter-devel&m=137132916826745&w=4
> 
> Having already spent over a day chasing this. Not having come across other cases on github except this person self reporting[1] made me think it must be indeed something in my code that I'm missing and that could have triggered this. Or is it really rare (harmless) bug in libnetfilter?

I guess this is the typical "problem" situation in which userspace uses
a non-zeroed buffer to feed into sendto() and due to padding not
every byte was written to. So basically userspace "leaks" garbage to
kernel, which is something I'd consider harmless and merely a minor
inconvenience when analyzing with valgrind. I usually suffer from this
as well since libmnl()'s allocation routines don't zero the buffer
either.

In your case, I'd say the error message disappears if you add
'memset(&u, 0, sizeof(u))' to the beginning of nfq_set_mode().

Cheers, Phil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: invalid read in
  2020-02-03 16:31 ` Phil Sutter
@ 2020-02-03 17:14   ` dyslexicatheist
  2020-02-03 17:30     ` Phil Sutter
  0 siblings, 1 reply; 5+ messages in thread
From: dyslexicatheist @ 2020-02-03 17:14 UTC (permalink / raw)
  To: netfilter-devel

On Monday, February 3, 2020 4:31 PM, Phil Sutter <phil@nwl.cc> wrote:

> Hi,
>
> On Mon, Feb 03, 2020 at 01:54:31PM +0000, dyslexicatheist wrote:
>
> I guess this is the typical "problem" situation in which userspace uses
> a non-zeroed buffer to feed into sendto() and due to padding not
> every byte was written to. So basically userspace "leaks" garbage to
> kernel, which is something I'd consider harmless and merely a minor
> inconvenience when analyzing with valgrind. I usually suffer from this
> as well since libmnl()'s allocation routines don't zero the buffer
> either.
>
> In your case, I'd say the error message disappears if you add
> 'memset(&u, 0, sizeof(u))' to the beginning of nfq_set_mode().

thanks for your help Phil. I have just tried this but unfortunately it didn't change the outcome. Also tried other variations such as memset'ing both &u and the &params struct, but nada.


>
> Cheers, Phil



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: invalid read in
  2020-02-03 17:14   ` dyslexicatheist
@ 2020-02-03 17:30     ` Phil Sutter
  2020-02-03 17:49       ` dyslexicatheist
  0 siblings, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2020-02-03 17:30 UTC (permalink / raw)
  To: dyslexicatheist; +Cc: netfilter-devel

Hi,

On Mon, Feb 03, 2020 at 05:14:45PM +0000, dyslexicatheist wrote:
> On Monday, February 3, 2020 4:31 PM, Phil Sutter <phil@nwl.cc> wrote:
> > On Mon, Feb 03, 2020 at 01:54:31PM +0000, dyslexicatheist wrote:
> >
> > I guess this is the typical "problem" situation in which userspace uses
> > a non-zeroed buffer to feed into sendto() and due to padding not
> > every byte was written to. So basically userspace "leaks" garbage to
> > kernel, which is something I'd consider harmless and merely a minor
> > inconvenience when analyzing with valgrind. I usually suffer from this
> > as well since libmnl()'s allocation routines don't zero the buffer
> > either.
> >
> > In your case, I'd say the error message disappears if you add
> > 'memset(&u, 0, sizeof(u))' to the beginning of nfq_set_mode().
> 
> thanks for your help Phil. I have just tried this but unfortunately it didn't change the outcome. Also tried other variations such as memset'ing both &u and the &params struct, but nada.

Maybe you need to apply the same to __build_send_cfg_msg() as well?

Cheers, Phil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: invalid read in
  2020-02-03 17:30     ` Phil Sutter
@ 2020-02-03 17:49       ` dyslexicatheist
  0 siblings, 0 replies; 5+ messages in thread
From: dyslexicatheist @ 2020-02-03 17:49 UTC (permalink / raw)
  To: netfilter-devel

On Monday, February 3, 2020 5:30 PM, Phil Sutter <phil@nwl.cc> wrote:

> > thanks for your help Phil. I have just tried this but unfortunately it didn't change the outcome. Also tried other variations such as memset'ing both &u and the &params struct, but nada.
>
> Maybe you need to apply the same to __build_send_cfg_msg() as well?

thanks, I just tried this. the error persists. this is puzzling.

>
> Cheers, Phil



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-03 13:54 invalid read in dyslexicatheist
2020-02-03 16:31 ` Phil Sutter
2020-02-03 17:14   ` dyslexicatheist
2020-02-03 17:30     ` Phil Sutter
2020-02-03 17:49       ` dyslexicatheist

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git