From 1b32947d3f6c19dc1716a27d50361dcf59c27882 Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Mon, 17 Mar 2008 21:37:40 +0000 Subject: [PATCH] * Aligned the semantics of the read_symlink() FS module hook with the readlink() function. It is no longer required to null-terminate the string, shall not fail, if the buffer is too small, and shall return the length of the string actually written into the buffer. * Adjusted rootfs, devfs, and bfs accordingly. Also adjusted their read_stat() hooks to return the correct symlink length in st_size. * Our readlink() does now comply to the standard (and BeOS). Additionally if the buffer is big enough it is nice to non-conforming apps and null-terminates it. * BSymLink::ReadLink() explicitly null-terminates the string now. git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@24425 a95241bf-73f2-0310-859d-f6bbb57e9c96 --- docs/user/drivers/fs_interface.dox | 23 +++++++++++-------- .../file_systems/bfs/kernel_interface.cpp | 19 +++++++-------- src/kits/storage/Entry.cpp | 3 ++- src/kits/storage/LibBeAdapter.cpp | 1 - src/kits/storage/SymLink.cpp | 13 ++++++++--- src/system/kernel/fs/devfs.cpp | 11 ++++----- src/system/kernel/fs/rootfs.cpp | 15 ++++++------ src/system/kernel/fs/vfs.cpp | 9 ++++++-- src/system/libroot/posix/unistd/link.c | 12 +++++++--- 9 files changed, 61 insertions(+), 45 deletions(-) diff --git a/docs/user/drivers/fs_interface.dox b/docs/user/drivers/fs_interface.dox index 2f3ed0ffeb..9d1eb0a1b7 100644 --- a/docs/user/drivers/fs_interface.dox +++ b/docs/user/drivers/fs_interface.dox @@ -1,9 +1,9 @@ /* - * Copyright 2007 Haiku Inc. All rights reserved. + * Copyright 2007-2008 Haiku Inc. All rights reserved. * Distributed under the terms of the MIT License. * * Authors: - * Ingo Weinhold + * Ingo Weinhold * Niels Sascha Reedijk * Axel Dörfler * @@ -536,21 +536,24 @@ fs_vnode link, char *buffer, size_t *_bufferSize) \brief Read the value of a symbolic link. - If the function is successful, the string written to the buffer shall be - null-terminated and the variable \a _bufferSize points to shall be set to - the length of that string, including the terminating null character. + If the function is successful, the symlink string shall be written to the + buffer. It does not need to be null-terminated. If the buffer is too small + to hold the complete string, only the first \c *_bufferSize bytes of the + string shall be written to the buffer; the buffer shall not be + null-terminated in this case. Furthermore the variable \a _bufferSize + points to shall be set to the length of the string written to the buffer, + not including any terminating null character (if written). \param fs The volume handle. \param link The node handle. \param buffer Pointer to a pre-allocated buffer the link value shall be written to. - \param buffer Pointer to a pre-allocated variable containing the size of the - buffer supplied to the function. Upon successful completion the hook shall - store the number of bytes actually written into the buffer in the variable. + \param _bufferSize Pointer to a pre-allocated variable containing the size + of the buffer supplied to the function. Upon successful completion the + hook shall store the number of bytes actually written into the buffer + in the variable. \retval B_OK Everything went fine. \retval B_BAD_VALUE \a link does not identify a symbolic link. - \retval B_BUFFER_OVERFLOW The supplied buffer is not big enough to contain - the complete link value. */ /*! diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp index 157630fcb9..c3c8f52381 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -49,7 +49,7 @@ fill_stat_buffer(Inode *inode, struct stat &stat) if (inode->IsSymLink() && (node.Flags() & INODE_LONG_SYMLINK) == 0) { // symlinks report the size of the link here - stat.st_size = strlen(node.short_symlink) + 1; + stat.st_size = strlen(node.short_symlink); } else stat.st_size = inode->Size(); } @@ -1366,29 +1366,26 @@ bfs_read_link(void *_ns, void *_node, char *buffer, size_t *_bufferSize) FUNCTION(); Inode *inode = (Inode *)_node; - size_t bufferSize = *_bufferSize; if (!inode->IsSymLink()) RETURN_ERROR(B_BAD_VALUE); if (inode->Flags() & INODE_LONG_SYMLINK) { - // we also need space for the terminating null byte - if (inode->Size() >= bufferSize) { - *_bufferSize = inode->Size() + 1; - return B_BUFFER_OVERFLOW; - } + if (inode->Size() < *_bufferSize) + *_bufferSize = inode->Size(); status_t status = inode->ReadAt(0, (uint8 *)buffer, _bufferSize); if (status < B_OK) RETURN_ERROR(status); - buffer[++*_bufferSize] = '\0'; return B_OK; } - *_bufferSize = strlcpy(buffer, inode->Node().short_symlink, bufferSize) + 1; - if (*_bufferSize > bufferSize) - return B_BUFFER_OVERFLOW; + size_t linkLen = strlen(inode->Node().short_symlink); + if (linkLen < *_bufferSize) + *_bufferSize = linkLen; + + memcpy(buffer, inode->Node().short_symlink, *_bufferSize); return B_OK; } diff --git a/src/kits/storage/Entry.cpp b/src/kits/storage/Entry.cpp index 3e74d0c19d..ddba1b0fe0 100644 --- a/src/kits/storage/Entry.cpp +++ b/src/kits/storage/Entry.cpp @@ -928,10 +928,11 @@ BEntry::set(int dirFD, const char *path, bool traverse) // we need to traverse the symlink if (--linkLimit < 0) return B_LINK_LIMIT; - size_t bufferSize = B_PATH_NAME_LENGTH; + size_t bufferSize = B_PATH_NAME_LENGTH - 1; error = _kern_read_link(dirFD, leafName, tmpPath, &bufferSize); if (error < 0) return error; + tmpPath[bufferSize] = '\0'; path = tmpPath; // next round... } diff --git a/src/kits/storage/LibBeAdapter.cpp b/src/kits/storage/LibBeAdapter.cpp index b0ea344695..aaacc06757 100644 --- a/src/kits/storage/LibBeAdapter.cpp +++ b/src/kits/storage/LibBeAdapter.cpp @@ -534,7 +534,6 @@ _kern_read_link(int fd, const char *path, char *buffer, size_t *_bufferSize) if (result < 0) return result; - buffer[result] = '\0'; *_bufferSize = result; return B_OK; } diff --git a/src/kits/storage/SymLink.cpp b/src/kits/storage/SymLink.cpp index 77e11322d0..c708325ade 100644 --- a/src/kits/storage/SymLink.cpp +++ b/src/kits/storage/SymLink.cpp @@ -93,7 +93,8 @@ BSymLink::~BSymLink() // ReadLink //! Reads the contents of the symbolic link into a buffer. -/*! \param buf the buffer +/*! The string written to the buffer will be null-terminated. + \param buf the buffer \param size the size of the buffer \return - the number of bytes written into the buffer @@ -110,11 +111,17 @@ BSymLink::ReadLink(char *buffer, size_t size) if (InitCheck() != B_OK) return B_FILE_ERROR; - status_t error = _kern_read_link(get_fd(), NULL, buffer, &size); + size_t linkLen = size; + status_t error = _kern_read_link(get_fd(), NULL, buffer, &linkLen); if (error < B_OK) return error; - return size; + // null-terminate + if (linkLen >= size) + return B_BUFFER_OVERFLOW; + buffer[linkLen] = '\0'; + + return linkLen; } // MakeLinkedPath diff --git a/src/system/kernel/fs/devfs.cpp b/src/system/kernel/fs/devfs.cpp index fae846e4a4..c90e01f643 100644 --- a/src/system/kernel/fs/devfs.cpp +++ b/src/system/kernel/fs/devfs.cpp @@ -1880,13 +1880,10 @@ devfs_read_link(fs_volume _fs, fs_vnode _link, char *buffer, size_t *_bufferSize if (!S_ISLNK(link->stream.type)) return B_BAD_VALUE; - *_bufferSize = link->stream.u.symlink.length + 1; - // we always need to return the number of bytes we intend to write! + if (link->stream.u.symlink.length < *_bufferSize) + *_bufferSize = link->stream.u.symlink.length; - if (bufferSize <= link->stream.u.symlink.length) - return B_BUFFER_OVERFLOW; - - memcpy(buffer, link->stream.u.symlink.path, link->stream.u.symlink.length + 1); + memcpy(buffer, link->stream.u.symlink.path, *_bufferSize); return B_OK; } @@ -2478,6 +2475,8 @@ devfs_read_stat(fs_volume _fs, fs_vnode _vnode, struct stat *stat) // is this a real block device? then let's have it reported like that if (stat->st_size != 0) stat->st_mode = S_IFBLK | (vnode->stream.type & S_IUMSK); + } else if (S_ISLNK(vnode->stream.type)) { + stat->st_size = vnode->stream.u.symlink.length; } return B_OK; diff --git a/src/system/kernel/fs/rootfs.cpp b/src/system/kernel/fs/rootfs.cpp index e001dd71d7..df78d3f284 100644 --- a/src/system/kernel/fs/rootfs.cpp +++ b/src/system/kernel/fs/rootfs.cpp @@ -805,18 +805,14 @@ static status_t rootfs_read_link(fs_volume _fs, fs_vnode _link, char *buffer, size_t *_bufferSize) { struct rootfs_vnode *link = (rootfs_vnode*)_link; - size_t bufferSize = *_bufferSize; if (!S_ISLNK(link->stream.type)) return B_BAD_VALUE; - *_bufferSize = link->stream.symlink.length + 1; - // we always need to return the number of bytes we intend to write! + if (link->stream.symlink.length < *_bufferSize) + *_bufferSize = link->stream.symlink.length; - if (bufferSize <= link->stream.symlink.length) - return B_BUFFER_OVERFLOW; - - memcpy(buffer, link->stream.symlink.path, link->stream.symlink.length + 1); + memcpy(buffer, link->stream.symlink.path, *_bufferSize); return B_OK; } @@ -975,7 +971,10 @@ rootfs_read_stat(fs_volume _fs, fs_vnode _v, struct stat *stat) // stream exists, but we know to return size 0, since we can only hold directories stat->st_dev = fs->id; stat->st_ino = vnode->id; - stat->st_size = 0; + if (S_ISLNK(vnode->stream.type)) + stat->st_size = vnode->stream.symlink.length; + else + stat->st_size = 0; stat->st_mode = vnode->stream.type; stat->st_nlink = 1; diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index c07c5b85bf..6b253fc7de 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -1780,9 +1780,13 @@ vnode_path_to_vnode(struct vnode *vnode, char *path, bool traverseLeafLink, } if (FS_CALL(nextVnode, read_symlink) != NULL) { + bufferSize--; status = FS_CALL(nextVnode, read_symlink)( nextVnode->mount->cookie, nextVnode->private_node, buffer, &bufferSize); + // null-terminate + if (status >= 0) + buffer[bufferSize] = '\0'; } else status = B_BAD_VALUE; @@ -7399,11 +7403,12 @@ _user_normalize_path(const char* userPath, bool traverseLink, char* buffer) // read link struct stat st; if (FS_CALL(fileVnode, read_symlink) != NULL) { - size_t bufferSize = B_PATH_NAME_LENGTH; + size_t bufferSize = B_PATH_NAME_LENGTH - 1; error = FS_CALL(fileVnode, read_symlink)(fileVnode->mount->cookie, fileVnode->private_node, path, &bufferSize); if (error != B_OK) return error; + path[bufferSize] = '\0'; } else return B_BAD_VALUE; } @@ -7704,7 +7709,7 @@ _user_read_link(int fd, const char *userPath, char *userBuffer, size_t *userBuff if (status < B_OK) return status; - if (user_strlcpy(userBuffer, buffer, bufferSize) < 0) + if (user_memcpy(userBuffer, buffer, bufferSize) != B_OK) return B_BAD_ADDRESS; return B_OK; diff --git a/src/system/libroot/posix/unistd/link.c b/src/system/libroot/posix/unistd/link.c index 92a420b3a8..6db0fff161 100644 --- a/src/system/libroot/posix/unistd/link.c +++ b/src/system/libroot/posix/unistd/link.c @@ -20,13 +20,19 @@ ssize_t readlink(const char *path, char *buffer, size_t bufferSize) { - status_t status = _kern_read_link(-1, path, buffer, &bufferSize); - if (status < B_OK && status != B_BUFFER_OVERFLOW) { + size_t linkLen = bufferSize; + status_t status = _kern_read_link(-1, path, buffer, &linkLen); + if (status < B_OK) { errno = status; return -1; } - return bufferSize; + // If the buffer is big enough, null-terminate the string. That's not + // required by the standard, but helps non-conforming apps. + if (linkLen < bufferSize) + buffer[linkLen] = '\0'; + + return linkLen; }