Re: [SCM] dpkg's main repository branch, master, updated. 1.16.0.3-191-ge135afd
Hi!
On Wed, 2011-07-20 at 06:43:51 +0000, Raphaël Hertzog wrote:
> The following commit has been merged in the master branch:
> commit e135afdb35d0ac179657def901965a448115a981
> Author: Raphaël Hertzog <hertzog@debian.org>
> Date: Wed Jul 20 08:29:05 2011 +0200
>
> dpkg: fix possible segfault in findbreakcycle().
>
> The circumstances are not entirely clear because clear_istobes() is
> called earlier in the code and should already ensure that clientdata
> is allocated for all packages in the database but the stack trace
> reported leaves no room for any other interpretation. We must protect
> the access to tpkg->clientdata in findbreakcycle() with
> ensure_package_clientdata(tpkg).
>
> Probably that some other parts of the code might create new packages in the
> in-memory database depending on some specific conditions. It might be that
> those conditions only hold for a multiarch-enabled dpkg for example if
> the code looks up a package entry for an alternative architecture and
> would thus create the package on the fly.
>
> This is pure speculation because I did not push the investigations that
> far. It might be something entirely different but it doesn't matter much
> because the proposed fix is the same and just ensures that we respect
> our API by protecting the access to clientdata.
>
> See https://bugs.launchpad.net/ubuntu/+source/dpkg/+bug/733414 for
> details.
Hmm, trying to "fix" a bug w/o understanding the causes is almost never
a good idea. In this case doubly so if this only happens in the Ubuntu
dpkg. The commit might as well just be shifting the segfaults to a later
point if other parts of the code are inserting new packages through
pkg_db_find() or in areas where findbreakcycle() is not being used. The
debian/changelog entry is also wrongly attributed.
This commit is not ok and should be reverted.
Something else I've been meaning to state lately is that I really want to
review and ack any patch touching the dpkg C code. And just to clarify,
not due to this specific commit, but it has just reminded me about it.
regards,
guillem
Reply to: