public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* RE: resolver //Was: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency
@ 2022-01-17 18:29 Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  2022-01-18 10:50 ` Corinna Vinschen
  0 siblings, 1 reply; 2+ messages in thread
From: Lavrentiev, Anton (NIH/NLM/NCBI) [C] @ 2022-01-17 18:29 UTC (permalink / raw)
  To: cygwin-patches

Hi Corinna,

> Other than that, the remaining patches look good, except, adding a short
> description what patch 7 does to the commit message would be great for
> later readers of the git log.

I resubmitted the patches with a little improvement and a better description
to the #7 (now #5) as requested.

While doing the code review afresh in there, I noticed a few more problems:

1.
minires-os-if.c on line 262 does this:
    262         ptr = NULL;
    263         break;

and then a bit later this:
    290   len = ptr - AnsPtr;

which makes the return value "len" to be a total nonsense (I think it should
return -1 in this case, but it's debatable).

2.
Also, "ptr" in the cygwin_query() function is not checked for buffer overrun
in the "done:" section of the code (after line 291), which is not good IMO.

3.
Lastly, at other places where "ptr" is checked for overrun (notably, in write_record()),
it can leave garbage in the unfilled portion of the answer buffer (because it still
advances "ptr" properly, to calculate the final "would-be" size of the response):
so if the return value is greater than the passed "AnsLength", the user cannot assume
that at least all AnsLength bytes were filled correctly.  They cannot actually assume
any "boundary" where the "Ans" buffer was actually stopped being updated.

Maybe "Ans" should be cleared upon entry?... But that would mean double-write of
the buffer in most of the use-cases (where no overflow actually occurs because of
an adequate size of the buffer).

Anton Lavrentiev
Contractor NIH/NLM/NCBI


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

end of thread, other threads:[~2022-01-18 10:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-17 18:29 resolver //Was: [PATCH 3/7] Debug output to show both IP and port # in native b.o., a few little cosmetic improvements for consistency Lavrentiev, Anton (NIH/NLM/NCBI) [C]
2022-01-18 10:50 ` Corinna Vinschen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).