Re: [PATCH] Fix harmless uninitialized read in svn_fs_fs__l2p_index_append

[Orivej Desh]

* Daniel Shahaf <d...@daniel.shahaf.name> [2020-07-27]
> Orivej Desh wrote on Sat, 25 Jul 2020 23:27 +0000:
> > Clang 10 memory sanitizer reports an uninitialized read of .offset in
> >      if ((entry > 0 && proto_entry.offset == 0) || eof)
> > when read_l2p_entry_from_proto_index set eof and left the proto_entry unset.
>
> Orivej Desh wrote on Sat, 25 Jul 2020 23:27 +0000:
> > [[[
> > Fix harmless uninitialized read in svn_fs_fs__l2p_index_append
> >
> > * subversion/libsvn_fs_fs/index.c
> >   (svn_fs_fs__l2p_index_append): Do not access proto_entry.offset when
> >   it is unset due to reaching eof.
>
> Well written.
>
> > ]]]
> > +++ subversion/libsvn_fs_fs/index.c (working copy)
> > @@ -827,7 +827,7 @@ svn_fs_fs__l2p_index_append(svn_checksum_t **check
> >        /* handle new revision */
> > -      if ((entry > 0 && proto_entry.offset == 0) || eof)
> > +      if (eof || (entry > 0 && proto_entry.offset == 0))
>
> Looks good to me, +1.
>
> Does libsvn_fs_x need the same change?

Indeed, I have fixed memory-sanitized "svnadmin create --fs-type fsx"
with the attached patch.

> Thanks for the patch,
>
> Daniel

[[[
Fix harmless uninitialized read in svn_fs_*_index_append

* subversion/libsvn_fs_fs/index.c (svn_fs_fs__l2p_index_append),
  subversion/libsvn_fs_x/index.c
  (svn_fs_x__l2p_index_append, svn_fs_x__p2l_index_append):
  Do not access entry fields that are unset due to reaching eof.
]]]
Index: subversion/libsvn_fs_fs/index.c
===================================================================
--- subversion/libsvn_fs_fs/index.c	(revision 1880343)
+++ subversion/libsvn_fs_fs/index.c	(working copy)
@@ -827,7 +827,7 @@ svn_fs_fs__l2p_index_append(svn_checksum_t **check
                                               &eof, local_pool));

       /* handle new revision */
-      if ((entry > 0 && proto_entry.offset == 0) || eof)
+      if (eof || (entry > 0 && proto_entry.offset == 0))
         {
           /* dump entries, grouped into pages */

Index: subversion/libsvn_fs_x/index.c
===================================================================
--- subversion/libsvn_fs_x/index.c	(revision 1880343)
+++ subversion/libsvn_fs_x/index.c	(working copy)
@@ -953,7 +953,7 @@ svn_fs_x__l2p_index_append(svn_checksum_t **checks
                                               &eof, local_pool));

       /* handle new revision */
-      if ((entry > 0 && proto_entry.offset == 0) || eof)
+      if (eof || (entry > 0 && proto_entry.offset == 0))
         {
           /* dump entries, grouped into pages */

@@ -2219,7 +2219,7 @@ svn_fs_x__p2l_index_append(svn_checksum_t **checks
       SVN_ERR(read_p2l_entry_from_proto_index(proto_index, &entry,
                                               &eof, iterpool));

-      if (entry.item_count && !eof)
+      if (!eof && entry.item_count)
         {
           entry.items = apr_palloc(iterpool,
                                    entry.item_count * sizeof(*entry.items));