[pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

newer
Re: [pacman-dev] December 74% OFF

older
[pacman-dev] [PATCH] Add remove...

Allan McRae

22 Dec 2007 22 Dec '07

4:06 a.m.

Attachments:

0001-Add-check-for-NULL-data-parameter-in-sync-commit.patch (text/x-patch — 1005 bytes)

Show replies by thread

Nagy Gabor

22 Dec 22 Dec

1:03 p.m.

New subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

...

From: Allan McRae To: Discussion list for pacman development Subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit Date: Sat, 22 Dec 2007 14:06:44 +1000 Reply-To: Discussion list for pacman development User-Agent: Thunderbird 2.0.0.9 (X11/20071213)

...
From ccb76f0d909f3532af8e4004a125d21f9012ae78 Mon Sep 17 00:00:00 2001 From: Allan McRae Date: Sat, 22 Dec 2007 14:01:48 +1000 Subject: [PATCH] Add check for NULL data parameter in sync commit

Fixes FS#7380: alpm crashes on passing NULL to alpm_trans_commit in a sync operation. Adds sanity check in _alpm_trans_commit for case PM_TRANS_TYPE_SYNC.

Signed-off-by: Allan McRae --- lib/libalpm/trans.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c index 582c76b..31b2747 100644 --- a/lib/libalpm/trans.c +++ b/lib/libalpm/trans.c @@ -415,6 +415,8 @@ int _alpm_trans_commit(pmtrans_t *trans, alpm_list_t **data) } break; case PM_TRANS_TYPE_SYNC: + /* Sanity checks */ + ASSERT(data != NULL, RET_ERR(PM_ERR_WRONG_ARGS, -1)); if(_alpm_sync_commit(trans, handle->db_local, data) == -1) { /* pm_errno is set by _alpm_sync_commit() */ return(-1);

Indeed, this bug still exists. But I interpret your patch as a workaround (or hotfix), right? I mean we should fix alpm_sync_commit to work with NULL **data (which means we don't need additional error info) Bye

Allan McRae

4:03 p.m.

New subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

Nagy Gabor wrote:

...

Indeed, this bug still exists. But I interpret your patch as a workaround (or hotfix), right? I mean we should fix alpm_sync_commit to work with NULL **data (which means we don't need additional error info)

Bye Yeah, it was really a hotfix. I hadn't thought about people wanting to ignore the error messages. Here is what alpm_trans_commit calls:

alpm_trans_commit -> _alpm_trans_commit -> _alpm_sync_commit -> test_delta_md5sum & test_pkg_md5sum -> test_md5sum -> _alpm_trans_prepare -> _alpm_sync_prepare -> _alpm_resolvedeps The only places I can find writing to data are without a if(data) block are: _alpm_trans_prepare _alpm_sync_prepare test_md5sum The attached patch fixes those and thus should fixed the problem at its source.

Nagy Gabor

9:11 p.m.

New subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

...

Yeah, it was really a hotfix. I hadn't thought about people wanting to ignore the error messages. Here is what alpm_trans_commit calls:

alpm_trans_commit -> _alpm_trans_commit -> _alpm_sync_commit -> test_delta_md5sum & test_pkg_md5sum -> test_md5sum -> _alpm_trans_prepare -> _alpm_sync_prepare -> _alpm_resolvedeps

The only places I can find writing to data are without a if(data) block are: _alpm_trans_prepare _alpm_sync_prepare test_md5sum

The attached patch fixes those and thus should fixed the problem at its source.

Great work! The patch has just eliminated the segfault. Thanks. Bye

Nagy Gabor

23 Dec 23 Dec

1:05 p.m.

New subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

...

diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c index 7661866..0a8170c 100644 --- a/lib/libalpm/sync.c +++ b/lib/libalpm/sync.c @@ -675,7 +675,9 @@ int _alpm_sync_prepare(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t *dbs_sync if(deps) { pm_errno = PM_ERR_UNSATISFIED_DEPS; ret = -1; - *data = deps; + if(data) { + *data = deps; + } goto cleanup; } } Some little comments: If you don't keep the result, you should free deps list with FREELIST().

And the same for the allocated errormsg strings <- here you can put the whole {calloc, snprintf, alpm_list_add} block to if(data). Bye

Allan McRae

24 Dec 24 Dec

12:56 p.m.

New subject: [pacman-dev] [PATCH] Add check for NULL data parameter in sync commit

Nagy Gabor wrote:

...

Some little comments: If you don't keep the result, you should free deps list with FREELIST().

And the same for the allocated errormsg strings <- here you can put the whole {calloc, snprintf, alpm_list_add} block to if(data).

Bye

Updated patch attached that fixes the above issues. Allan

5989

Age (days ago)

5991

Last active (days ago)

List overview

Download

5 comments

3 participants

participants (3)

Allan McRae
Allan McRae
Nagy Gabor