vdr-1.4.1: bug report and fix: ReadLink with relative targets
Commit Message
Patrick Cernko wrote:
> Hi again,
>
> Patrick Cernko wrote:
>> Hi Klaus, hi list,
>>
>> today I discovered that the ReadLink function (used in e.g. cSafeFile)
>> does not handle relative links correctly. I used a symlinked
>> channels.conf like that:
>>
>> errror@ds9:/var/lib/vdr> ll channels.conf
>> lrwxrwxrwx 1 errror vdr 20 2006-06-12 00:03 channels.conf ->
>> channels.conf.normal
>>
>> but vdr tried to write to channels.conf.$$$ in its current working
>> directory ("/") which failed. :-(
>>
>> The bug is caused by the ReadLink function which always took the
>> unchanged value of the found link as target (without prepending the
>> links path for relative links).
>>
>> As a fix, I added a check for relative links (== not starting with a
>> '/') and prepend the directory of the symlink. For that purpose I use
>> some small parts from the coreutils "dirname" program as I learned for a
>> long time that doing something so easy as computing the directory part
>> of a path can lead to several errors if not done right. :-)
>>
>> Appended you find the patch, which makes vdr handle relative symlinks
>> correctly.
>>
>> @Klaus: Feel free to review/reduce the code but pay attention to the
>> special cases! ;-)
Well, your patch looks like overkill to me ;-)
What about using 'canonicalize_file_name()' instead?
See the attached patch.
Klaus
Comments
Klaus Schmidinger wrote:
> Patrick Cernko wrote:
>> Hi again,
>>
>>>
>>> @Klaus: Feel free to review/reduce the code but pay attention to the
>>> special cases! ;-)
>
> Well, your patch looks like overkill to me ;-)
>
> What about using 'canonicalize_file_name()' instead?
>
Hmm, sorry, it seems you missed declaration and definition of
canonicalize_file_name or is it a (system) function my sarge system
doesn't know of (in that case, which header file do I have to include)?
Generally I fully agree with you: If you find a simple implementation,
that ensures, that readlink-values not starting with '/' are prepended
with dirname(symlink) followed by a "/", just do it.
So long,
P.S.: I'm currently in vacation in Turkey and do not read my mails very
regulary.
Patrick Cernko wrote:
> Klaus Schmidinger wrote:
>> Patrick Cernko wrote:
>>> Hi again,
>>>
>>>> @Klaus: Feel free to review/reduce the code but pay attention to the
>>>> special cases! ;-)
>> Well, your patch looks like overkill to me ;-)
>>
>> What about using 'canonicalize_file_name()' instead?
>>
>
> Hmm, sorry, it seems you missed declaration and definition of
> canonicalize_file_name or is it a (system) function my sarge system
> doesn't know of (in that case, which header file do I have to include)?
>
Intresting I do not find any documentation for canonicalize_file_name
beside the small comment in stdlib.h ! This seems to be the smartest
solution. I agree fully with you.
So long,
Patrick Cernko wrote:
> Patrick Cernko wrote:
>> Klaus Schmidinger wrote:
>>> Patrick Cernko wrote:
>>>> Hi again,
>>>>
>>>>> @Klaus: Feel free to review/reduce the code but pay attention to the
>>>>> special cases! ;-)
>>> Well, your patch looks like overkill to me ;-)
>>>
>>> What about using 'canonicalize_file_name()' instead?
>>>
>> Hmm, sorry, it seems you missed declaration and definition of
>> canonicalize_file_name or is it a (system) function my sarge system
>> doesn't know of (in that case, which header file do I have to include)?
>>
>
> Intresting I do not find any documentation for canonicalize_file_name
> beside the small comment in stdlib.h ! This seems to be the smartest
> solution. I agree fully with you.
The function is documented in "The GNU C Library Reference Manual".
Works just fine here.
Klaus
@@ -480,22 +480,16 @@
char *ReadLink(const char *FileName)
{
- char RealName[PATH_MAX];
- const char *TargetName = NULL;
- int n = readlink(FileName, RealName, sizeof(RealName) - 1);
- if (n < 0) {
- if (errno == ENOENT || errno == EINVAL) // file doesn't exist or is not a symlink
- TargetName = FileName;
+ if (!FileName)
+ return NULL;
+ char *TargetName = canonicalize_file_name(FileName);
+ if (!TargetName) {
+ if (errno == ENOENT) // file doesn't exist
+ TargetName = strdup(FileName);
else // some other error occurred
LOG_ERROR_STR(FileName);
}
- else if (n < int(sizeof(RealName))) { // got it!
- RealName[n] = 0;
- TargetName = RealName;
- }
- else
- esyslog("ERROR: symlink's target name too long: %s", FileName);
- return TargetName ? strdup(TargetName) : NULL;
+ return TargetName;
}
bool SpinUpDisk(const char *FileName)