Rewrite dnames

[masjerang]

In dvbloopback.h, there is a enum referring to names of the devices, used in print/debug statements. During a commit of #18 the patch is not correctly applied, so here is a patch to correct it. For descrambler we probably need a similar patch. I'll look into it as well.

diff --git a/dvbloopback/dvbloopback.h b/dvbloopback/dvbloopback.h
index df004aa..645274b 100644
--- a/dvbloopback/dvbloopback.h
+++ b/dvbloopback/dvbloopback.h
@@ -6,8 +6,7 @@
 #define DVBLB_MAXFD 96

 static const char * const dnames[] = {
-        "video", "audio", "sec", "frontend", "demux", "dvr", "ca",
-         "sec", "frontend", "demux", "dvr", "ca", "net", "video", "audio", "osd" "sec", "frontend", "demux", "dvr", "ca", "net", "video", "audio", "osd"
+        "sec", "frontend", "demux", "dvr", "ca", "net", "video", "audio", "osd"
 };

 typedef enum dvblb_type {

See dvbdev.h, the enum dvb_device_type for reference.

[masjerang]

I probably need to setup a proper copy so that I can use the pull request feature, but here is another patch to sanitize the logging output a bit:

diff --git a/dvbloopback/dvb_loopback.c b/dvbloopback/dvb_loopback.c
index 6b6d7b0..557da3c 100644
--- a/dvbloopback/dvb_loopback.c
+++ b/dvbloopback/dvb_loopback.c
@@ -288,8 +288,8 @@ static int dvblb_fake_ioctl(struct dvblb_devinfo *lbdev, struct dvb_device **f,
        while(1) {
                ret = wait_event_interruptible_timeout(lbdev->wait_ioctl,
                         lbdev->ioctlcmd == ULONG_MAX, HZ);
-               dprintk2("fake-ioctl wait (%lu) returned: %d/%lu\n", cmd,
-                        ret, lbdev->ioctlcmd);
+               dprintk2("fake-ioctl wait (%lu) returned: %d/%lu\n", 0xFF & cmd,
+                        ret, 0xFF & lbdev->ioctlcmd);
                if(ret >= 0 && lbdev->ioctlcmd == ULONG_MAX)
                        break;
                if(cmd == DVBLB_CMD_CLOSE) {
@@ -356,7 +356,7 @@ static int dvblb_open(struct inode *inode, struct file *f)
                printk("Failed to find private data during open\n");
                return -EFAULT;
        }
-       dprintk("dvblb_open %d%s%d\n", lbdev->parent->adapter.num,
+       dprintk("dvblb_open adapter%d/%s%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type], dvbdev->id);

        if(dvbdev->id == 0) {
@@ -452,7 +452,7 @@ static int dvblb_release(struct inode *inode, struct file *f)
                printk("Failed to find private data during close\n");
                return -EFAULT;
        }
-       dprintk("dvblb_release %d%s%d fd:%d\n", lbdev->parent->adapter.num,
+       dprintk("dvblb_release adapter%d/%s%d fd:%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type],
                dvbdev->id, find_filemap(lbdev, filemap));

@@ -527,7 +527,7 @@ static ssize_t dvblb_write(struct file *f, const char *buf,
                printk("Failed to find private data during close\n");
                return -EFAULT;
        }
-       dprintk3("dvblb_write %d%s%d fd:%d\n", lbdev->parent->adapter.num,
+       dprintk3("dvblb_write adapter%d/%s%d fd:%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type],
                dvbdev->id, find_filemap(lbdev, filemap));

@@ -560,7 +560,7 @@ static ssize_t dvblb_read (struct file *f, char * buf, size_t count, loff_t *off
                printk("Failed to find private data during read\n");
                return -EFAULT;
        }
-       dprintk3("dvblb_read %d%s%d fd:%d\n", lbdev->parent->adapter.num,
+       dprintk3("dvblb_read adapter%d/%s%d fd:%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type],
                dvbdev->id, find_filemap(lbdev, filemap));
        if(dvbdev->id == 0) {
@@ -683,7 +683,7 @@ static int dvblb_looped_ioctl(struct file *f,
                 printk("Failed to find private data during ioctl\n");
                 return -EFAULT;
         }
-       dprintk("dvblb_ioctl %d%s%d fd:%d\n", lbdev->parent->adapter.num,
+       dprintk("dvblb_ioctl adapter%d/%s%d fd:%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type],
                dvbdev->id, find_filemap(lbdev, filemap));
        ret = dvblb_fake_ioctl(lbdev, filemap, cmd, parg);
@@ -723,9 +723,9 @@ static long dvblb_ioctl(struct file *f,
                                       dvbdev->kernel_ioctl);
        }
        /* This is the userspace control device */
-       dprintk2("dvblb_ioctl %d%s%d fd:%d cmd:%x\n",lbdev->parent->adapter.num,
+       dprintk2("dvblb_ioctl adapter%d/%s%d fd:%d cmd:%u\n",lbdev->parent->adapter.num,
                dnames[dvbdev->type],
-               dvbdev->id, find_filemap(lbdev, filemap), cmd);
+               dvbdev->id, find_filemap(lbdev, filemap), 0xFF & cmd);
        if (cmd == DVBLB_CMD_ASYNC) {
                /* async command return */
                int pos, i;
@@ -784,6 +784,7 @@ static long dvblb_ioctl(struct file *f,
        }
        if (cmd == FE_GET_PROPERTY)
        {    
+           int i;
            tvps = (struct dtv_properties __user *)(lbdev->ioctlretdata + sizeof(int));    
            if (copy_from_user(lbdev->ioctlretdata + size, tvps->props,
                         tvps->num * sizeof(struct dtv_property))) 
@@ -793,7 +794,9 @@ static long dvblb_ioctl(struct file *f,
            }
            tvps = (struct dtv_properties __user *)(lbdev->ioctlretdata + sizeof(int));
            tvps->props = (struct dtv_property __user *)(lbdev->ioctlretdata + size);
-           dprintk("%s() copy_from_user: cmd %u\n", __func__, tvps->props[0].cmd);
+           for (i = 0; i < tvps->num ; i++) { 
+                   dprintk("%s() copy_from_user: FE_GET_PROPERTY %d/%d cmd %u\n", __func__, i+1, tvps->num, tvps->props[i].cmd);
+               } 
        }

        lbdev->ioctlretval = *(int *)lbdev->ioctlretdata;
@@ -826,7 +829,7 @@ static int dvblb_mmap(struct file *f, struct vm_area_struct *vma)
                printk("Failed to find private data during mmap\n");
                return -EFAULT;
        }
-       dprintk("dvblb_mmap %d%s%d fd:%d\n", lbdev->parent->adapter.num,
+       dprintk("dvblb_mmap adapter%d/%s%d fd:%d\n", lbdev->parent->adapter.num,
                dnames[dvbdev->type],
                dvbdev->id, find_filemap(lbdev, filemap));
        if(dvbdev->id == 0) {
@@ -893,7 +896,7 @@ static unsigned int dvblb_poll(struct file *f, struct poll_table_struct *wait)
                printk("Failed to find private data during poll\n");
                return -EFAULT;
        }
-       dprintk3("dvblb_poll %d%s%d: fd:%d\n", lbdev->parent->adapter.num,
+       dprintk3("dvblb_poll adapter%d/%s%d: fd:%d\n", lbdev->parent->adapter.num,
                 dnames[dvbdev->type],
                 dvbdev->id, find_filemap(lbdev, filemap));
        if(dvbdev->id == 0) {
@@ -947,7 +950,7 @@ static unsigned int dvblb_poll(struct file *f, struct poll_table_struct *wait)
                        printk("skipping poll on %p (%d)\n", f, pos);
                        */
                }
-       dprintk3("dvblb_poll %d%s%d: fd:%d returned: %d\n",
+       dprintk3("dvblb_poll adapter%d/%s%d: fd:%d returned: %d\n",
                 lbdev->parent->adapter.num,
                 dnames[dvbdev->type], dvbdev->id, pos, ci.u.mode);
                return(ci.u.mode);

Please let me know what you think of it.

[Strunzdesign]

Currently the HEAD creates crashes on my system, probably caused by the dnames variable. Is it safe to apply these patches, or do I need to modify descrambler as well? Is a patch of descrambler required or just optional?

Regards, Florian

[Strunzdesign]

Yes, the first patch alone is sufficient to stop the stack traces. Thanks... but this should be committed to the repo!

[masjerang]

@Strunzdesign thanks for your comments. I'm using kernel 5.4.0 (Ubuntu Focal kernel) and haven't tested it with more recent kernels. I made the changes to dnames as I discovered the wrong interface names were printed. I was not really aware that those were used elsewhere than just debug print, but they are in creating things in the proc filesystem. So in HEAD dnames needs to be returned to what is was before, or apply the patch in this ticket. Are you able to check that my thinking is correct regarding changing dnames to what is in this ticket?

The same needs to be applied to descrabler binary. Dvbloopback.h is also used there.

The second patch is also to make the debug statement more meaningful.

[masjerang]

@bas-t can you apply the patch in the first comment? In the commit of #18 , some lines got not properly patched. This patch corrects this. See above communication.

[bas-t]

Please get a clone of this repo and give me a pull request. That way we should be able to avoid copy & paste errors.

Op zo 23 aug. 2020 13:32 schreef masjerang notifications@github.com:

@bas-t https://github.com/bas-t can you apply the patch in the first comment? In the commit of #18 https://github.com/bas-t/dvbloopback/issues/18 , some lines got not properly patched. This patch corrects this. See above communication.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bas-t/dvbloopback/issues/19#issuecomment-678762870, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF6DRSPDF4HJMBDONLDICLSCD45RANCNFSM4PK3MRNA .

[masjerang]

Pull-request #20

[masjerang]

@Strunzdesign master has been updated. Can you please verify if works on your end again? Thanks!

[Strunzdesign]

I'm back now and am going to conduct a test... as soon as my system update completes.

[Strunzdesign]

On my system the interface naming is fixed (current master on Linux kernel 5.8.8). However, I id not recompile or patch descrambler which might cause another clash somehow... you mentioned that files were changed that are part of descrambler as well... we should not forget to update that also.

[masjerang]

Thanks @Strunzdesign Descrambler and dvbloopback are independent for this change. Although descrambler needs a similar change. I’ll look into that and close this ticket.

Regarding descrambler, as a userspace program, I don’t like to have the kernel patched (ie DMX stuff and ca_pid), so it needs some more investigation.