technocloud-public/nginx-unit
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Don't conflate the error variable in nxt_kqueue_poll().

Browse Source 
In nxt_kqueue_poll() error is declared as a nxt_bool_t aka unsigned int
(on x86-64 anyway).

It is used both as a boolean and as the return storage for a bitwise AND
operation.

This has potential to go awry.

If nxt_bool_t was changed to be a u8 then we would have the following
issue

gcc12 -c -pipe -fPIC -fvisibility=hidden -O -W -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wmissing-prototypes -Werror -g -O2 -I src -I build     -I/usr/local/include  -o build/src/nxt_kqueue_engine.o  -MMD -MF build/src/nxt_kqueue_engine.dep -MT build/src/nxt_kqueue_engine.o  src/nxt_kqueue_engine.c
src/nxt_kqueue_engine.c: In function 'nxt_kqueue_poll':
src/nxt_kqueue_engine.c:728:17: error: overflow in conversion from 'int' to 'nxt_bool_t' {aka 'unsigned char'} changes value from '(int)kev->flags & 16384' to '0' [-Werror=overflow]
  728 |         error = (kev->flags & EV_ERROR);
      |                 ^
cc1: all warnings being treated as errors

EV_ERROR has the value 16384, after the AND operation error holds 16384,
however this overflows and wraps around (64 times) exactly to 0.

With nxt_bool_t defined as a u32, we would have a similar issue if
EV_ERROR ever became UINT_MAX + 1 (or a multiple thereof)...

Rather than conflating the use of error, keep error as a boolean (it is
used further down the function) but do the AND operation inside the
if ().

Reviewed-by: Alejandro Colomar <alx@nginx.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
This commit is contained in:
Andrew Clayton
2023-03-24 14:23:06 +00:00
parent b9177d36e7
commit 375556f9aa
1 changed files with 4 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
src/nxt_kqueue_engine.c
View File

@@ -716,6 +716,8 @@ nxt_kqueue_poll(nxt_event_engine_t *engine, nxt_msec_t timeout)
for (i = 0; i < nevents; i++) { for (i = 0; i < nevents; i++) {
error = 0;
kev = &engine->u.kqueue.events[i]; kev = &engine->u.kqueue.events[i];
nxt_debug(&engine->task, nxt_debug(&engine->task,
@@ -725,12 +727,11 @@ nxt_kqueue_poll(nxt_event_engine_t *engine, nxt_msec_t timeout)
kev->ident, kev->filter, kev->flags, kev->fflags, kev->ident, kev->filter, kev->flags, kev->fflags,
kev->data, kev->udata); kev->data, kev->udata);
error = (kev->flags & EV_ERROR); if (nxt_slow_path(kev->flags & EV_ERROR)) {
if (nxt_slow_path(error)) {
nxt_alert(&engine->task, nxt_alert(&engine->task,
"kevent(%d) error %E on ident:%d filter:%d", "kevent(%d) error %E on ident:%d filter:%d",
engine->u.kqueue.fd, kev->data, kev->ident, kev->filter); engine->u.kqueue.fd, kev->data, kev->ident, kev->filter);
error = 1;
} }
task = &engine->task; task = &engine->task;