FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian dpkg

 
 
LinkBack Thread Tools
 
Old 07-25-2011, 04:54 AM
Guillem Jover
 
Default 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


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20110725045445.GA4481@gaara.hadrons.org">http://lists.debian.org/20110725045445.GA4481@gaara.hadrons.org
 
Old 07-25-2011, 07:05 AM
Raphael Hertzog
 
Default dpkg's main repository branch, master, updated. 1.16.0.3-191-ge135afd

Hi,

On Mon, 25 Jul 2011, Guillem Jover wrote:
> 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.

Only if there are other places where we are incorrectly assuming that
clientdata is allocated without calling ensure_package_clientdata(). This
commit is fixing one of those places, why should we revert it?

If pkg_db_find must not create new packages on the fly, we should
consider changing its API instead. But I don't think that it's really
reasonable to expect some parts of the code to not have the right to use
pkg_db_find() because we fear that it might introduce new package entries.

So IMO the right course of action is to protect the access to clientdata
in particular when we have a segfault that proves that the initial
analysis that lead us to believe it's not needed is wrong.

> This commit is not ok and should be reverted.

What's the right course of action then?

Ignoring the problem because you don't know what part of the code
introduces the new package entry doesn't seem very reasonable.

I mean I can try to spend a couple of hours trying to find this but it
doesn't seem a very productive use of my time when the unprotected access
is so self-evident.

Cheers,
--
Rapha√ęl Hertzog ‚óą Debian Developer

Follow my Debian News ‚Ė∂ http://RaphaelHertzog.com (English)
‚Ė∂ http://RaphaelHertzog.fr (Fran√ßais)


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20110725070525.GD7758@rivendell.home.ouaza.com">ht tp://lists.debian.org/20110725070525.GD7758@rivendell.home.ouaza.com
 

Thread Tools




All times are GMT. The time now is 10:09 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org