* [PATCH 1/1] Fix use of vgname and vgid [not found] <cover.1303302985.git.zkabelac@redhat.com> @ 2011-04-20 12:37 ` Zdenek Kabelac 2011-04-20 12:38 ` Zdenek Kabelac 2011-04-20 21:36 ` Alasdair G Kergon 0 siblings, 2 replies; 3+ messages in thread From: Zdenek Kabelac @ 2011-04-20 12:37 UTC (permalink / raw) To: lvm-devel Avoid using of free memory when duplicated MDA is found. As get_pv_from_vg_by_id() may call lvmcache_label_scan() keep the vgname and vgid copied localy on the stack as vginfo may dissapear and code was then accessing garbage in memory. i.e. pvs /dev/loop0 (when /dev/loop0 and /dev/loop1 has same MDA content) Invalid read of size 1 at 0x523C986: dm_hash_lookup (hash.c:325) by 0x440C8C: vginfo_from_vgname (lvmcache.c:399) by 0x4605C0: _create_vg_text_instance (format-text.c:1882) by 0x46140D: _text_create_text_instance (format-text.c:2243) by 0x47EB49: _vg_read (metadata.c:2887) by 0x47FBD8: vg_read_internal (metadata.c:3231) by 0x477594: get_pv_from_vg_by_id (metadata.c:344) by 0x45F07A: _get_pv_if_in_vg (format-text.c:1400) by 0x45F0B9: _populate_pv_fields (format-text.c:1414) by 0x45F40F: _text_pv_read (format-text.c:1493) by 0x480431: _pv_read (metadata.c:3500) by 0x4802B2: pv_read (metadata.c:3462) Address 0x652ab80 is 0 bytes inside a block of size 4 free'd at 0x4C2756E: free (vg_replace_malloc.c:366) by 0x442277: _free_vginfo (lvmcache.c:963) by 0x44235E: _drop_vginfo (lvmcache.c:992) by 0x442B23: _lvmcache_update_vgname (lvmcache.c:1165) by 0x443449: lvmcache_update_vgname_and_id (lvmcache.c:1358) by 0x443C07: lvmcache_add (lvmcache.c:1492) by 0x46588C: _text_read (text_label.c:271) by 0x466A65: label_read (label.c:289) by 0x4413FC: lvmcache_label_scan (lvmcache.c:635) by 0x4605AD: _create_vg_text_instance (format-text.c:1881) by 0x46140D: _text_create_text_instance (format-text.c:2243) by 0x47EB49: _vg_read (metadata.c:2887) Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com> --- lib/format_text/format-text.c | 21 +++++++++++++++++---- 1 files changed, 17 insertions(+), 4 deletions(-) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index b8b4fc1..8d3b531 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -1396,10 +1396,23 @@ static int _get_pv_if_in_vg(struct lvmcache_info *info, struct physical_volume *pv) { if (info->vginfo && info->vginfo->vgname && - !is_orphan_vg(info->vginfo->vgname) && - get_pv_from_vg_by_id(info->fmt, info->vginfo->vgname, - info->vginfo->vgid, info->dev->pvid, pv)) - return 1; + !is_orphan_vg(info->vginfo->vgname)) { + char vgname[NAME_LEN + 1]; + char vgid[ID_LEN + 1]; + + /* + * get_pv_from_vg_by_id() may lead to + * lvmcache_label_scan() so it can drop + * vginfo and all referenced data. + */ + strcpy(vgname, info->vginfo->vgname); + vgname[PATH_MAX] = '\0'; + memcpy(vgid, info->vginfo->vgid, sizeof(vgid)); + + if (get_pv_from_vg_by_id(info->fmt, vgname, vgid, + info->dev->pvid, pv)) + return 1; + } return 0; } -- 1.7.4.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 1/1] Fix use of vgname and vgid 2011-04-20 12:37 ` [PATCH 1/1] Fix use of vgname and vgid Zdenek Kabelac @ 2011-04-20 12:38 ` Zdenek Kabelac 2011-04-20 21:36 ` Alasdair G Kergon 1 sibling, 0 replies; 3+ messages in thread From: Zdenek Kabelac @ 2011-04-20 12:38 UTC (permalink / raw) To: lvm-devel Dne 20.4.2011 14:37, Zdenek Kabelac napsal(a): > Avoid using of free memory when duplicated MDA is found. > > As get_pv_from_vg_by_id() may call lvmcache_label_scan() keep the vgname > and vgid copied localy on the stack as vginfo may dissapear and code was > then accessing garbage in memory. > > i.e. pvs /dev/loop0 > (when /dev/loop0 and /dev/loop1 has same MDA content) > > Invalid read of size 1 > at 0x523C986: dm_hash_lookup (hash.c:325) > by 0x440C8C: vginfo_from_vgname (lvmcache.c:399) > by 0x4605C0: _create_vg_text_instance (format-text.c:1882) > by 0x46140D: _text_create_text_instance (format-text.c:2243) > by 0x47EB49: _vg_read (metadata.c:2887) > by 0x47FBD8: vg_read_internal (metadata.c:3231) > by 0x477594: get_pv_from_vg_by_id (metadata.c:344) > by 0x45F07A: _get_pv_if_in_vg (format-text.c:1400) > by 0x45F0B9: _populate_pv_fields (format-text.c:1414) > by 0x45F40F: _text_pv_read (format-text.c:1493) > by 0x480431: _pv_read (metadata.c:3500) > by 0x4802B2: pv_read (metadata.c:3462) > Address 0x652ab80 is 0 bytes inside a block of size 4 free'd > at 0x4C2756E: free (vg_replace_malloc.c:366) > by 0x442277: _free_vginfo (lvmcache.c:963) > by 0x44235E: _drop_vginfo (lvmcache.c:992) > by 0x442B23: _lvmcache_update_vgname (lvmcache.c:1165) > by 0x443449: lvmcache_update_vgname_and_id (lvmcache.c:1358) > by 0x443C07: lvmcache_add (lvmcache.c:1492) > by 0x46588C: _text_read (text_label.c:271) > by 0x466A65: label_read (label.c:289) > by 0x4413FC: lvmcache_label_scan (lvmcache.c:635) > by 0x4605AD: _create_vg_text_instance (format-text.c:1881) > by 0x46140D: _text_create_text_instance (format-text.c:2243) > by 0x47EB49: _vg_read (metadata.c:2887) > > Signed-off-by: Zdenek Kabelac <zkabelac@redhat.com> > --- > lib/format_text/format-text.c | 21 +++++++++++++++++---- > 1 files changed, 17 insertions(+), 4 deletions(-) > > diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c > index b8b4fc1..8d3b531 100644 > --- a/lib/format_text/format-text.c > +++ b/lib/format_text/format-text.c > @@ -1396,10 +1396,23 @@ static int _get_pv_if_in_vg(struct lvmcache_info *info, > struct physical_volume *pv) > { > if (info->vginfo && info->vginfo->vgname && > - !is_orphan_vg(info->vginfo->vgname) && > - get_pv_from_vg_by_id(info->fmt, info->vginfo->vgname, > - info->vginfo->vgid, info->dev->pvid, pv)) > - return 1; > + !is_orphan_vg(info->vginfo->vgname)) { > + char vgname[NAME_LEN + 1]; > + char vgid[ID_LEN + 1]; > + > + /* > + * get_pv_from_vg_by_id() may lead to > + * lvmcache_label_scan() so it can drop > + * vginfo and all referenced data. > + */ > + strcpy(vgname, info->vginfo->vgname); > + vgname[PATH_MAX] = '\0'; ^^^^^ ignore > + memcpy(vgid, info->vginfo->vgid, sizeof(vgid)); > + > + if (get_pv_from_vg_by_id(info->fmt, vgname, vgid, > + info->dev->pvid, pv)) > + return 1; > + } > > return 0; > } ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] Fix use of vgname and vgid 2011-04-20 12:37 ` [PATCH 1/1] Fix use of vgname and vgid Zdenek Kabelac 2011-04-20 12:38 ` Zdenek Kabelac @ 2011-04-20 21:36 ` Alasdair G Kergon 1 sibling, 0 replies; 3+ messages in thread From: Alasdair G Kergon @ 2011-04-20 21:36 UTC (permalink / raw) To: lvm-devel On Wed, Apr 20, 2011 at 02:37:07PM +0200, Zdenek Kabelac wrote: > As get_pv_from_vg_by_id() may call lvmcache_label_scan() keep the vgname > and vgid copied localy on the stack as vginfo may dissapear and code was > then accessing garbage in memory. Currently, vginfo structs must not be held across calls to vg_read (or used as parameters). Have you audited the code to check there are no similar problems elsewhere? I'm not too happy with the workaround in this patch, but it'll do for now, as a proper fix would take quite a bit longer to develop. > + /* > + * get_pv_from_vg_by_id() may lead to > + * lvmcache_label_scan() so it can drop > + * vginfo and all referenced data. > + */ Document the restriction I mentioned above at the vg_read* entry point. Alasdair ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-04-20 21:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1303302985.git.zkabelac@redhat.com>
2011-04-20 12:37 ` [PATCH 1/1] Fix use of vgname and vgid Zdenek Kabelac
2011-04-20 12:38 ` Zdenek Kabelac
2011-04-20 21:36 ` Alasdair G Kergon
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.