Re: [PATCH 4/4] Fix ForwardEngineOutputToUser overflow behavior.

--0000000000000cd441063ce22318
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Rudolf,

Thank you very much for your patches. They fix the problems you reported
indeed and
will be included in the coming release 6.3.0.

Best regards,
Antonio Ceballos

On Mon, Jun 2, 2025 at 3:30=E2=80=AFPM <[email protected]> wrote:

> From: Rudolf Polzer <[email protected]>
>
> The previous implementation read 4097 bytes, but always cleared the
> 4097th byte - so whenever the engine had more than 4096 bytes to send,
> one byte will get spuriously zeroed out, leading to lichess-bot
> complaining about `inf\x00` being an undefined UCI command and similar
> things.
>
> In theory this can even lead to the engine timing out and losing a game
> as the UCI command with the actual move has been corrupted, but I have
> never observed that happen.
>
> As here all data corruption happens within the buffer, this cannot be
> reproduced with AddressSanitizer.
> ---
>  src/frontend/engine.cc | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/src/frontend/engine.cc b/src/frontend/engine.cc
> index e8fb3a2..cb7d974 100644
> --- a/src/frontend/engine.cc
> +++ b/src/frontend/engine.cc
> @@ -551,7 +551,7 @@ void ForwardEngineOutputToUser( void )
>    fd_set set[1];
>    struct timeval time_val[1];
>    int engineinputready=3D0;
> -  char engineinputaux[BUF_SIZE+1]=3D"";
> +  char engineinputaux[BUF_SIZE]=3D"";
>
>    /* Poll input from engine in non-blocking mode */
>    FD_ZERO(set);
> @@ -564,15 +564,13 @@ void ForwardEngineOutputToUser( void )
>      printf( "Error reading engine input.\n" );
>    } else if ( engineinputready > 0 ) {
>      /* There are some data from the engine. Read the data */
> -    strncpy( engineinputaux, zerochar, BUF_SIZE+1 );
> -    nread =3D read( pipefd_e2a[0], engineinputaux, BUF_SIZE+1 );
> -    /* Write data to output */
> -    assert( nread <=3D BUF_SIZE+1 );
> -    if (nread < BUF_SIZE+1) {
> -      engineinputaux[nread] =3D '\0';
> -    } else {
> -      engineinputaux[BUF_SIZE] =3D '\0';
> +    nread =3D read( pipefd_e2a[0], engineinputaux, BUF_SIZE );
> +    if ( nread =3D=3D -1 ) {
> +      printf( "Error reading message from engine.\n" );
> +      return;
>      }
> +    /* Write data to output */
> +    assert( nread <=3D BUF_SIZE );
>      ssize_t r =3D write( STDOUT_FILENO, engineinputaux, nread );
>      if ( r =3D=3D -1 ) {
>        printf( "Error sending message to engine.\n" );
> --
> 2.39.5
>
>
>

--0000000000000cd441063ce22318
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div class=3D"gmail_default" style=3D"font-size:small">Hi =
Rudolf,</div><div class=3D"gmail_default" style=3D"font-size:small"><br></d=
iv><div class=3D"gmail_default" style=3D"font-size:small">Thank you very mu=
ch for your patches. They fix the problems you reported indeed and</div><di=
v class=3D"gmail_default" style=3D"font-size:small">will be included in the=
 coming release 6.3.0.</div><div class=3D"gmail_default" style=3D"font-size=
:small"><br></div><div class=3D"gmail_default" style=3D"font-size:small">Be=
st regards,</div><div class=3D"gmail_default" style=3D"font-size:small">Ant=
onio Ceballos</div></div><br><div class=3D"gmail_quote gmail_quote_containe=
r"><div dir=3D"ltr" class=3D"gmail_attr">On Mon, Jun 2, 2025 at 3:30=E2=80=
=AFPM &lt;<a href=3D"mailto:[email protected]">[email protected]</a>&gt=
; wrote:<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px=
 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: R=
udolf Polzer &lt;<a href=3D"mailto:[email protected]" target=3D"_blank">d=
[email protected]</a>&gt;<br>
<br>
The previous implementation read 4097 bytes, but always cleared the<br>
4097th byte - so whenever the engine had more than 4096 bytes to send,<br>
one byte will get spuriously zeroed out, leading to lichess-bot<br>
complaining about `inf\x00` being an undefined UCI command and similar<br>
things.<br>
<br>
In theory this can even lead to the engine timing out and losing a game<br>
as the UCI command with the actual move has been corrupted, but I have<br>
never observed that happen.<br>
<br>
As here all data corruption happens within the buffer, this cannot be<br>
reproduced with AddressSanitizer.<br>
---<br>
=C2=A0src/frontend/engine.cc | 16 +++++++---------<br>
=C2=A01 file changed, 7 insertions(+), 9 deletions(-)<br>
<br>
diff --git a/src/frontend/engine.cc b/src/frontend/engine.cc<br>
index e8fb3a2..cb7d974 100644<br>
--- a/src/frontend/engine.cc<br>
+++ b/src/frontend/engine.cc<br>
@@ -551,7 +551,7 @@ void ForwardEngineOutputToUser( void )<br>
=C2=A0 =C2=A0fd_set set[1];<br>
=C2=A0 =C2=A0struct timeval time_val[1];<br>
=C2=A0 =C2=A0int engineinputready=3D0;<br>
-=C2=A0 char engineinputaux[BUF_SIZE+1]=3D&quot;&quot;;<br>
+=C2=A0 char engineinputaux[BUF_SIZE]=3D&quot;&quot;;<br>
<br>
=C2=A0 =C2=A0/* Poll input from engine in non-blocking mode */<br>
=C2=A0 =C2=A0FD_ZERO(set);<br>
@@ -564,15 +564,13 @@ void ForwardEngineOutputToUser( void )<br>
=C2=A0 =C2=A0 =C2=A0printf( &quot;Error reading engine input.\n&quot; );<br=
>
=C2=A0 =C2=A0} else if ( engineinputready &gt; 0 ) {<br>
=C2=A0 =C2=A0 =C2=A0/* There are some data from the engine. Read the data *=
/<br>
-=C2=A0 =C2=A0 strncpy( engineinputaux, zerochar, BUF_SIZE+1 );<br>
-=C2=A0 =C2=A0 nread =3D read( pipefd_e2a[0], engineinputaux, BUF_SIZE+1 );=
<br>
-=C2=A0 =C2=A0 /* Write data to output */<br>
-=C2=A0 =C2=A0 assert( nread &lt;=3D BUF_SIZE+1 );<br>
-=C2=A0 =C2=A0 if (nread &lt; BUF_SIZE+1) {<br>
-=C2=A0 =C2=A0 =C2=A0 engineinputaux[nread] =3D &#39;\0&#39;;<br>
-=C2=A0 =C2=A0 } else {<br>
-=C2=A0 =C2=A0 =C2=A0 engineinputaux[BUF_SIZE] =3D &#39;\0&#39;;<br>
+=C2=A0 =C2=A0 nread =3D read( pipefd_e2a[0], engineinputaux, BUF_SIZE );<b=
r>
+=C2=A0 =C2=A0 if ( nread =3D=3D -1 ) {<br>
+=C2=A0 =C2=A0 =C2=A0 printf( &quot;Error reading message from engine.\n&qu=
ot; );<br>
+=C2=A0 =C2=A0 =C2=A0 return;<br>
=C2=A0 =C2=A0 =C2=A0}<br>
+=C2=A0 =C2=A0 /* Write data to output */<br>
+=C2=A0 =C2=A0 assert( nread &lt;=3D BUF_SIZE );<br>
=C2=A0 =C2=A0 =C2=A0ssize_t r =3D write( STDOUT_FILENO, engineinputaux, nre=
ad );<br>
=C2=A0 =C2=A0 =C2=A0if ( r =3D=3D -1 ) {<br>
=C2=A0 =C2=A0 =C2=A0 =C2=A0printf( &quot;Error sending message to engine.\n=
&quot; );<br>
-- <br>
2.39.5<br>
<br>
<br>
</blockquote></div>

--0000000000000cd441063ce22318--