Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

READ_STAT_DONE flag is not set on empty reads #4

Open
debuggerpls opened this issue May 29, 2021 · 3 comments
Open

READ_STAT_DONE flag is not set on empty reads #4

debuggerpls opened this issue May 29, 2021 · 3 comments
Assignees

Comments

@debuggerpls
Copy link

READ_STAT_DONE flag is not set when vcon_read is called with size of 0.
Example: CONS frontends stop outputting data even when nothing is read, because it checks for input with size=0.

--- a/l4re/util/include/vcon_svr
+++ b/l4re/util/include/vcon_svr
@@ -87,7 +87,7 @@ public:
     l4_umword_t v = this_vcon()->vcon_read(buf, size);
     unsigned bytes = v & L4_VCON_READ_SIZE_MASK;

-    if (bytes < size)
+    if (bytes < size || (!bytes && !size))
       v |= L4_VCON_READ_STAT_DONE;

     m->mr[0] = v;
@icedieler
Copy link
Contributor

Hi,
thanks for reporting this issue. Could you elaborate a little bit more on this issue? I am not sure what you mean when you say it "stops outputting data when nothing is read".

Thanks,
Matthias.

@icedieler icedieler self-assigned this May 31, 2021
@debuggerpls
Copy link
Author

If the previous comment was not clear, I meant that L4_VCON_READ_STAT_DONE flag in class L4Re::Util::Vcon_svr is not set on empty reads. As per Vcon_svr documentation:

* vcon_read() needs to update the status argument with the
* L4_vcon_read_stat flags, especially the L4_VCON_READ_STAT_DONE flag
* to indicate that there's nothing more to read for the other end.

In the case when size=0 and nothing is read (bytes=0) using vcon_read(), the L4_VCON_READ_STAT_DONE flag would be not set, thus failing to indicate that there is nothing more to read.

l4_umword_t v = this_vcon()->vcon_read(buf, size);      
unsigned bytes = v & L4_VCON_READ_SIZE_MASK;

if (bytes < size)
  v |= L4_VCON_READ_STAT_DONE;

kk-infra pushed a commit that referenced this issue Oct 26, 2023
Closes ticket #4.
http://www.uclibc-ng.org/trac/ticket/4

Change-Id: I543c9da5afae8de3d02334b72dceb368db225ce4
kk-infra pushed a commit that referenced this issue Jul 17, 2024
- use the provided __res_state() method instead of direct access
  to struct __res_state pointer &_res/*__resp

- change the __UCLIBC_HAS_TLS__ protected __res_state() implementation
  to the one where the comment 'When threaded, _res may be a per-thread
  variable.' indicates this should be used with threads/TLS enabled

Fixes the following segfaults with buildroot raspberrypi3_64_defconfig
(uclibc, -Os, Note: runs fine using the raspberrypi3_defconfig):

  $ /usr/sbin/ntpd -n -d
  1970-01-01T00:01:49 ntpd[249]: INIT: ntpd ntpsec-1.2.0 2021-11-03T20:39:50Z: Starting
  1970-01-01T00:01:49 ntpd[249]: INIT: Command line: /usr/sbin/ntpd -n -d
  1970-01-01T00:01:49 ntpd[249]: INIT: precision = 7.240 usec (-17)
  1970-01-01T00:01:49 ntpd[249]: INIT: successfully locked into RAM
  1970-01-01T00:01:49 ntpd[249]: CONFIG: readconfig: parsing file: /etc/ntp.conf
  1970-01-01T00:01:49 ntpd[249]: CONFIG: restrict nopeer ignored
  1970-01-01T00:01:49 ntpd[249]: INIT: Using SO_TIMESTAMPNS
  1970-01-01T00:01:49 ntpd[249]: IO: Listen and drop on 0 v6wildcard [::]:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listen and drop on 1 v4wildcard 0.0.0.0:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listen normally on 2 lo 127.0.0.1:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listen normally on 3 eth0 172.16.0.30:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listen normally on 4 lo [::1]:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listen normally on 5 eth0 [fe80::ba27:ebff:fea6:340%2]:123
  1970-01-01T00:01:49 ntpd[249]: IO: Listening on routing socket on fd #22 for interface updates
  1970-01-01T00:01:50 ntpd[249]: SYNC: Found 10 servers, suggest minsane at least 3
  1970-01-01T00:01:50 ntpd[249]: INIT: MRU 10922 entries, 13 hash bits, 65536 bytes
  1970-01-01T00:01:50 ntpd[249]: statistics directory /var/NTP/ does not exist or is unwriteable, error No such file or directory
  1970-01-01T00:01:51 ntpd[249]: DNS: dns_probe: 0.pool.ntp.org, cast_flags:8, flags:101
  Segmentation fault (core dumped)

  $ ./host/bin/aarch64-buildroot-linux-uclibc-gdb ./build/ntpsec-1_2_0/build/main/ntpd/ntpd core
  Core was generated by `/usr/sbin/ntpd -n -d'.
  Program terminated with signal SIGSEGV, Segmentation fault.
  (gdb) where
  #0  0x0000007f8ff1f150 in res_sync_func () at libc/inet/resolv.c:3356
  #1  0x0000007f8ff1c468 in __open_nameservers () at libc/inet/resolv.c:949
  #2  0x0000007f8ff1b498 in __dns_lookup (name=0x55943c67f0 "0.pool.ntp.org",
      type=1, outpacket=0x7f8fe91c48, a=0x7f8fe91c08) at libc/inet/resolv.c:1134
  #3  0x0000007f8ff1d744 in __GI_gethostbyname_r (
      name=0x55943c67f0 "0.pool.ntp.org", result_buf=0x7f8fe92628,
      buf=0x7f8fe91d90 "", buflen=992, result=0x7f8fe92670,
      h_errnop=0x7f8fe92668) at libc/inet/resolv.c:1966
  #4  0x0000007f8ff1d9a0 in __GI_gethostbyname2_r (
      name=0x55943c67f0 "0.pool.ntp.org", family=2, result_buf=0x7f8fe92628,
      buf=0x7f8fe91d70 "0.pool.ntp.org", buflen=1024, result=0x7f8fe92670,
      h_errnop=0x7f8fe92668) at libc/inet/resolv.c:2065
  #5  0x0000007f8ff16924 in gaih_inet (name=0x55943c67f0 "0.pool.ntp.org",
      service=0x7f8fe92828, req=0x7f8fe92890, pai=0x7f8fe92838)
      at libc/inet/getaddrinfo.c:596
  #6  0x0000007f8ff17624 in __GI_getaddrinfo (
      name=0x55943c67f0 "0.pool.ntp.org",
      service=0x5582eb8acd "\377H\213D$\bL\211\367H\213\260\270",
      hints=0x7f8fe92890, pai=0x5582ee1bf8) at libc/inet/getaddrinfo.c:957
  #7  0x0000005582ea60f4 in _start ()
  (gdb) p _res
  $1 = {options = 0, nsaddr_list = {{sin_family = 0, sin_port = 0, sin_addr = {
          s_addr = 0}, sin_zero = "\000\000\000\000\000\000\000"}, {
        sin_family = 0, sin_port = 0, sin_addr = {s_addr = 0},
        sin_zero = "\000\000\000\000\000\000\000"}, {sin_family = 0,
        sin_port = 0, sin_addr = {s_addr = 0},
        sin_zero = "\000\000\000\000\000\000\000"}}, dnsrch = {0x0, 0x0, 0x0,
      0x0, 0x0, 0x0, 0x0}, nscount = 0 '\000', ndots = 0 '\000',
    retrans = 0 '\000', retry = 0 '\000', defdname = '\000' <repeats 255 times>,
    nsort = 0 '\000', pfcode = 0, id = 0, res_h_errno = 0, sort_list = {{addr = {
          s_addr = 0}, mask = 0}, {addr = {s_addr = 0}, mask = 0}, {addr = {
          s_addr = 0}, mask = 0}, {addr = {s_addr = 0}, mask = 0}, {addr = {
          s_addr = 0}, mask = 0}, {addr = {s_addr = 0}, mask = 0}, {addr = {
          s_addr = 0}, mask = 0}, {addr = {s_addr = 0}, mask = 0}, {addr = {
          s_addr = 0}, mask = 0}, {addr = {s_addr = 0}, mask = 0}}, _u = {
      _ext = {nsaddrs = {0x0, 0x0, 0x0}, nscount = 0 '\000', nstimes = {0, 0,
          0}, nssocks = {0, 0, 0}, nscount6 = 0, nsinit = 0}}}
  (gdb) p &_res
  $2 = (struct __res_state *) 0x7f8ff8fd98 <_res>
  (gdb) p rp
  $3 = (struct __res_state *) 0x7fffffffff

  And the following uclibc code at libc/inet/resolv.c:3356:

  3345 static void res_sync_func(void)
  3346 {
  3347         struct __res_state *rp = &(_res);
  3348         int n;
  3349
  3350         /* If we didn't get malloc failure earlier... */
  3351         if (__nameserver != (void*) &__local_nameserver) {
  3352                 /* TODO:
  3353                  * if (__nameservers < rp->nscount) - try to grow __nameserver[]?
  3354                  */
  3355 #ifdef __UCLIBC_HAS_IPV6__
  3356                 if (__nameservers > rp->_u._ext.nscount)
  3357                         __nameservers = rp->_u._ext.nscount;
  3358                 n = __nameservers;

  The special thing about ntpsec is the DNS lookup in an extra thread
  and/or the call to res_init(), see ntpsec-1_2_0/ntpd/ntp_dns.c:

   69         msyslog(LOG_INFO, "DNS: dns_probe: %s, cast_flags:%x, flags:%x%s",
   70                 hostname, pp->cast_flags, pp->cfg.flags, busy);
   71         if (NULL != active)     /* normally redundant */
   72                 return false;
   73
   74         active = pp;
   75
   76         sigfillset(&block_mask);
   77         pthread_sigmask(SIG_BLOCK, &block_mask, &saved_sig_mask);
   78         rc = pthread_create(&worker, NULL, dns_lookup, pp);

  and

  165 static void* dns_lookup(void* arg)
  166 {
  167         struct peer *pp = (struct peer *) arg;
  168         struct addrinfo hints;
  169
  170 #ifdef HAVE_SECCOMP_H
  171         setup_SIGSYS_trap();      /* enable trap for this thread */
  172 #endif
  173
  174 #ifdef HAVE_RES_INIT
  175         /* Reload DNS servers from /etc/resolv.conf in case DHCP has updated it.
  176          * We only need to do this occasionally, but it's not expensive
  177          * and simpler to do it every time than it is to figure out when
  178          * to do it.
  179          * This res_init() covers NTS too.
  180          */
  181         res_init();
  182 #endif
  183
  184         if (pp->cfg.flags & FLAG_NTS) {
  185 #ifndef DISABLE_NTS
  186                 nts_probe(pp);
  187 #endif
  188         } else {
  189                 ZERO(hints);
  190                 hints.ai_protocol = IPPROTO_UDP;
  191                 hints.ai_socktype = SOCK_DGRAM;
  192                 hints.ai_family = AF(&pp->srcadr);
  193                 gai_rc = getaddrinfo(pp->hostname, NTP_PORTA, &hints, &answer);
  194         }

  $ /usr/lib/uclibc-ng-test/test/inet/tst-res
  Segmentation fault (core dumped)

  $ ./host/bin/aarch64-buildroot-linux-uclibc-gdb ./build/uclibc-ng-test-0844445e7358eb10e716155b55b0fb23e88d644a/test/inet/tst-res core
  Core was generated by `/usr/lib/uclibc-ng-test/test/inet/tst-res'.
  Program terminated with signal SIGSEGV, Segmentation fault.
  (gdb) where
  #0  __GI___res_init () at libc/inet/resolv.c:3514
  #1  0x0000005591e507e4 in main (argc=<optimized out>, argv=<optimized out>)
      at tst-res.c:20

First reported here:
https://lore.kernel.org/buildroot/[email protected]/
https://www.mail-archive.com/[email protected]/msg01085.html

Change-Id: I646dd9dc45be812e82f00cda7227992dcaf41930
Signed-off-by: Peter Seiderer <[email protected]>
kk-infra pushed a commit that referenced this issue Jul 17, 2024
O0 build result in the following codegen

00000000 <ldrex>:
   0:	b480      	push	{r7}
   2:	b085      	sub	sp, #20
   4:	af00      	add	r7, sp, #0
   6:	6078      	str	r0, [r7, #4]
   8:	687b      	ldr	r3, [r7, #4]
   a:	e853 3f00 	ldrex	r3, [r3]
   e:	60fb      	str	r3, [r7, #12]
  10:	68fb      	ldr	r3, [r7, #12]
  12:	4618      	mov	r0, r3
  14:	3714      	adds	r7, #20
  16:	46bd      	mov	sp, r7
  18:	f85d 7b04 	ldr.w	r7, [sp], #4
  1c:	4770      	bx	lr

0000001e <strex>:
  1e:	b480      	push	{r7}
  20:	b085      	sub	sp, #20
  22:	af00      	add	r7, sp, #0
  24:	6078      	str	r0, [r7, #4]
  26:	6039      	str	r1, [r7, #0]
  28:	687b      	ldr	r3, [r7, #4]
  2a:	683a      	ldr	r2, [r7, #0]
  2c:	e842 3300 	strex	r3, r3, [r2]
  30:	60fb      	str	r3, [r7, #12]
  32:	68fb      	ldr	r3, [r7, #12]
  34:	4618      	mov	r0, r3
  36:	3714      	adds	r7, #20
  38:	46bd      	mov	sp, r7
  3a:	f85d 7b04 	ldr.w	r7, [sp], #4
  3e:	4770      	bx	lr

00000040 <testandset>:
  40:	b590      	push	{r4, r7, lr}
  42:	b083      	sub	sp, #12
  44:	af00      	add	r7, sp, #0
  46:	6078      	str	r0, [r7, #4]
  48:	6878      	ldr	r0, [r7, #4]
  4a:	f7ff fffe 	bl	0 <ldrex>
  4e:	4603      	mov	r3, r0
  50:	461c      	mov	r4, r3
  52:	6879      	ldr	r1, [r7, #4]
  54:	2001      	movs	r0, #1
  56:	f7ff fffe 	bl	1e <strex>
  5a:	4603      	mov	r3, r0
  5c:	2b00      	cmp	r3, #0
  5e:	d1f3      	bne.n	48 <testandset+0x8>
  60:	4623      	mov	r3, r4
  62:	4618      	mov	r0, r3
  64:	370c      	adds	r7, #12
  66:	46bd      	mov	sp, r7
  68:	bd90      	pop	{r4, r7, pc}

ARM ARM suggests that LoadExcl/StoreExcl loops are guaranteed to make
forward progress only if, for any LoadExcl/StoreExcl loop within a
single thread of execution, the software meets all of the following
conditions:

1 Between the Load-Exclusive and the Store-Exclusive, there are no
  explicit memory accesses, preloads, direct or indirect System
  register writes, address translation instructions, cache or TLB
  maintenance instructions, exception generating instructions,
  exception returns, or indirect branches.

...

Obviously condition is not met for O0 builds.

O2 build (which is highly likely the most common setting) able to do
the right thing resulting in

00000000 <ldrex>:
   0:	e850 0f00 	ldrex	r0, [r0]
   4:	4770      	bx	lr
   6:	bf00      	nop

00000008 <strex>:
   8:	e841 0000 	strex	r0, r0, [r1]
   c:	4770      	bx	lr
   e:	bf00      	nop

00000010 <testandset>:
  10:	2101      	movs	r1, #1
  12:	4603      	mov	r3, r0
  14:	e853 0f00 	ldrex	r0, [r3]
  18:	e843 1200 	strex	r2, r1, [r3]
  1c:	2a00      	cmp	r2, #0
  1e:	d1f9      	bne.n	14 <testandset+0x4>
  20:	4770      	bx	lr
  22:	bf00      	nop

Rather than depending on level of optimisation implement whole
ldrex/strex loop in inline assembly.

Change-Id: I00c64902124b5d8638225266ed6f946e225aa27c
Signed-off-by: Vladimir Murzin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants