Opened 12 years ago

Closed 12 years ago

#4825 closed defect (fixed)

check readlink for errors

Reported by: Erik Hovland <erik@…> Owned by: Nigel
Priority: minor Milestone: 0.22
Component: mythtv Version: 0.21-fixes
Severity: low Keywords:
Cc: Ticket locked: no

Description

The libc call readlink() is a bit tricky. It should be checked to make sure that nothing strange happened. In the member function MediaMonitorUnix::AddDevice?() it is checked to see if anything was written to it. But it is not checked for errors.

Attachments (1)

libs_libmyth_mediamonitor-unix.cpp-check-readlink.patch (853 bytes) - added by Erik Hovland <erik@…> 12 years ago.
adds a conditional to make really sure readlink worked properly

Download all attachments as: .zip

Change History (5)

Changed 12 years ago by Erik Hovland <erik@…>

adds a conditional to make really sure readlink worked properly

comment:1 Changed 12 years ago by Isaac Richards

Milestone: 0.210.22

comment:2 Changed 12 years ago by Nigel

Owner: changed from Isaac Richards to Nigel
Status: newassigned

I'm still looking at the logic in that method, but the patch will return from it without adding the device. I suspect the change should be just to ignore all errors. e.g.:

% svn diff libs/libmyth/mediamonitor-unix.cpp
Index: libs/libmyth/mediamonitor-unix.cpp
===================================================================
--- libs/libmyth/mediamonitor-unix.cpp  (revision 16346)
+++ libs/libmyth/mediamonitor-unix.cpp  (working copy)
@@ -465,9 +465,11 @@
     struct fstab * mep = NULL;
     char lpath[PATH_MAX];
 
-    // Resolve the simlink for the device.
+    // Resolve the symlink for the device.
     int len = readlink(devicePath, lpath, PATH_MAX);
-    if (len > 0 && len < PATH_MAX)
+    if (len == -1)
+        len = 0;
+    if (len < PATH_MAX)
         lpath[len] = 0;
 
     // Attempt to open the file system descriptor entry.
@@ -495,7 +497,7 @@
 
         // Check to see if this is the same as our passed in device.
         if ((strcmp(devicePath, mep->fs_spec) != 0) &&
-             (len && (strcmp(lpath, mep->fs_spec) != 0)))
+            (strncmp(lpath, mep->fs_spec, PATH_MAX) != 0))
             continue;
     }

comment:3 Changed 12 years ago by Erik Hovland <erik@…>

Your patch should do the job. I am rebuilding and checking to make sure the uninitialized lpath goes away. Thanks for taking a look.

comment:4 Changed 12 years ago by Nigel

Resolution: fixed
Status: assignedclosed

(In [16412]) Check for errors from readlink(), based on patch by Erik Hovland. Closes #4825. (also cope with unlikely situation of string overflow)

Note: See TracTickets for help on using tickets.