[linux-lvm] Lvdisplay crashes: Bug ???

[ahuja at austin.ibm.com]

I tried to post this on lvm-devel but seemingly the website for
registration seems to be down so posting it here. I hope some of the lvm
developers will be looking at this mailing list as well.

To recreate:

1. If necessary, create a physical volume, a volume group, and a logical
volume.
2. Create a snapshot of the logical volume.
3. Run 'lvdisplay' on either the original logical volume or its snapshot
lv.

Actual Results:
With 2.4.19 (shipped with SLES 8 base install), the kernel panics.  With
2.4.21-80, messages about bad vfree's appear /var/log/messages.  lvdisplay
sometimes segfaults, sometimes works, but the messages show up regardless.

Expected Results:
Successful completion of lvdisplay without panics or vfree messages or
segfaults.


Here is the sequence of commands I use to reproduce the problem:
# pvcreate /dev/sda4
# vgcreate test_vg /dev/sda4
# lvcreate -L 500M -n test_lv test_vg
# lvcreate -s -L 100M -n test_snap_lv /dev/test_vg/test_lv
# lvdisplay /dev/test_vg/test_lv


Probable cause:

In vg_free.c

vg_free
nt vg_free ( vg_t *vg, int all) {
   int l = 0;
   int p = 0;
   int ret = 0;

   debug_enter ( "vg_free Manish-1- CALLED\n");

   if ( vg == NULL || ( all != TRUE && all != FALSE)) ret = -LVM_EPARAM;
   else {
      debug ( "vg_free -- entering PV loop\n");
      for ( p = 0; p < vg->pv_cur; p++) {
         if ( vg->pv[p] != NULL) {
            if ( vg->pv[p]->pe != NULL)
                        free ( vg->pv[p]->pe);

                ^^^^^^^^^ this fails.


Tracking it down to the earlier call, the ptr values seem to be copied
along with the whole structure. Since this maybe a bad idea. Possibly
fixing that will fix the problem.

static int lvm_do_pv_status(vg_t *vg_ptr, void *arg, unsigned long size)
...
...
 if (copy_to_user(pv_status_req.pv, pv_ptr, size) != 0)
                                return -EFAULT;
....


pv_t structure has the following members and a copy_to_user may be a bad
idea here ??

typedef struct pv_v2 {
        char id[2];             /* Identifier */
        unsigned short version; /*
	.....
	.....
	pe_disk_t *pe;          /* HM */
	.....
}pv_t


Thoughts & suggestions.