On Thu, Oct 17, 2024 at 1:40 PM Thomas Huth <th...@redhat.com> wrote:
> On 16/10/2024 18.22, Daniel P. Berrangé wrote:
> > On Wed, Oct 16, 2024 at 06:07:12PM +0200, Thomas Huth wrote:
> >> The linker on OpenBSD complains:
> >>
> >> ld: warning: dirtyrate.c:447 (../src/migration/dirtyrate.c:447)(...):
> >> warning: strcpy() is almost always misused, please use strlcpy()
> >
> > Is that the only place it complains ? We use 'strcpy' in almost
> > 100 places across the codebase....
>
> There are only a fistful of other warnings. I guess most of the spots are
> turned into inlined code by the compiler, so the linker never sees those
> other occurrences.
>
> >> It's currently not a real problem in this case since both arrays
> >> have the same size (256 bytes). But just in case somebody changes
> >> the size of the source array in the future, let's better play safe
> >> and use g_strlcpy() here instead.
> >>
> >> Signed-off-by: Thomas Huth <th...@redhat.com>
> >> ---
> >> migration/dirtyrate.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> >> index 233acb0855..090c76e934 100644
> >> --- a/migration/dirtyrate.c
> >> +++ b/migration/dirtyrate.c
> >> @@ -444,7 +444,7 @@ static void get_ramblock_dirty_info(RAMBlock *block,
> >> info->ramblock_pages = qemu_ram_get_used_length(block) >>
> >> qemu_target_page_bits();
> >> info->ramblock_addr = qemu_ram_get_host_addr(block);
> >> - strcpy(info->idstr, qemu_ram_get_idstr(block));
> >> + g_strlcpy(info->idstr, qemu_ram_get_idstr(block),
> sizeof(info->idstr));
> >> }
> >
> > Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
> >
> >
> > Is it worth also adding
> >
> > G_STATIC_ASSERT(sizeof((struct RamblockDirtyInfo){}.idstr) ==
> > sizeof((struct RAMBlock){}.idstr));
> >
> > at the top of this file, since both of these fields are expected to
> > be the same size by this code, to avoid truncation.
>
> ... or alternatively check the return value of g_strlcpy() ? ... but that
> wouldn't work if pstrcpy() if we switch to that function instead.
>
> I don't mind either way - Peter, Fabiano, Hyman, what's your opinion here?
>
> Thomas
>
>
Since RamblockDirtyInfo is only used in dirtyrate.c, I'm inclined to just
check the return value of g_strlcpy to avoid DoS attack, and fix this
warning case by case.
Thanks,
Yong
--
Best regards
