fs: fs_mount parameter verification before mutex locked block

The commit moves fs_mount parameter verification above mutex lock.
The list of mount points is now checked before attempting to obtain
file system API pointer.
All modifications to mount point data structure, given as a parameter
to the fs_mount, are only applied after every other operation needed
have completed successfully, immediately before adding the mount point
to the list of mount points.
The fs_mount will a warning when mounted file system does not support
unmount.
When a file system does not provide mount function, the -ENOTSUP error
will be returned instead of -EINVAL.

Signed-off-by: Dominik Ermel <dominik.ermel@nordicsemi.no>
This commit is contained in:
Dominik Ermel 2020-10-15 13:12:41 +00:00 committed by Carles Cufí
commit 5faf3750bc
3 changed files with 45 additions and 31 deletions

View file

@ -392,10 +392,17 @@ int fs_closedir(struct fs_dir_t *zdp);
* calling the file system specific mount function and adding
* the mount point to mounted file system list.
*
* @param mp Pointer to the fs_mount_t structure
* @param mp Pointer to the fs_mount_t structure. Referenced object
* is not changed if the mount operation failed.
* A reference is captured in the fs infrastructure if the
* mount operation succeeds, and the application must not
* mutate the structure contents until fs_unmount is
* successfully invoked on the same pointer.
*
* @retval 0 on success;
* @retval <0 a negative errno code on error.
* @retval -ENOENT when file system type has not been registered;
* @retval -ENOTSUP when not supported by underlying file system driver;
* @retval <0 a other negative errno code on error.
*/
int fs_mount(struct fs_mount_t *mp);

View file

@ -581,14 +581,40 @@ int fs_mount(struct fs_mount_t *mp)
const struct fs_file_system_t *fs;
sys_dnode_t *node;
int rc = -EINVAL;
size_t len = 0;
/* Do all the mp checks prior to locking the mutex on the file
* subsystem.
*/
if ((mp == NULL) || (mp->mnt_point == NULL)) {
LOG_ERR("mount point not initialized!!");
return -EINVAL;
}
len = strlen(mp->mnt_point);
if ((len <= 1) || (mp->mnt_point[0] != '/')) {
LOG_ERR("invalid mount point!!");
return -EINVAL;
}
k_mutex_lock(&mutex, K_FOREVER);
/* Check if mount point already exists */
SYS_DLIST_FOR_EACH_NODE(&fs_mnt_list, node) {
itr = CONTAINER_OF(node, struct fs_mount_t, node);
/* continue if length does not match */
if (len != itr->mountp_len) {
continue;
}
if (strncmp(mp->mnt_point, itr->mnt_point, len) == 0) {
LOG_ERR("mount point already exists!!");
rc = -EBUSY;
goto mount_err;
}
}
/* Get file system information */
fs = fs_type_get(mp->type);
if (fs == NULL) {
@ -597,48 +623,27 @@ int fs_mount(struct fs_mount_t *mp)
goto mount_err;
}
mp->mountp_len = strlen(mp->mnt_point);
if ((mp->mnt_point[0] != '/') ||
(strlen(mp->mnt_point) <= 1)) {
LOG_ERR("invalid mount point!!");
rc = -EINVAL;
goto mount_err;
}
if (fs->mount == NULL) {
LOG_ERR("fs ops functions not set!!");
rc = -EINVAL;
LOG_ERR("fs type %d does not support mounting", mp->type);
rc = -ENOTSUP;
goto mount_err;
}
/* Check if mount point already exists */
SYS_DLIST_FOR_EACH_NODE(&fs_mnt_list, node) {
itr = CONTAINER_OF(node, struct fs_mount_t, node);
/* continue if length does not match */
if (mp->mountp_len != itr->mountp_len) {
continue;
if (fs->unmount == NULL) {
LOG_WRN("mount path %s is not unmountable",
log_strdup(mp->mnt_point));
}
if (strncmp(mp->mnt_point, itr->mnt_point,
mp->mountp_len) == 0) {
LOG_ERR("mount Point already exists!!");
rc = -EBUSY;
goto mount_err;
}
}
rc = fs->mount(mp);
if (rc < 0) {
LOG_ERR("fs mount error (%d)", rc);
goto mount_err;
}
/* set mount point fs interface */
/* Update mount point data and append it to the list */
mp->mountp_len = len;
mp->fs = fs;
/* append to the mount list */
sys_dlist_append(&fs_mnt_list, &mp->node);
LOG_DBG("fs mounted at %s", log_strdup(mp->mnt_point));

View file

@ -295,11 +295,13 @@ static int temp_statvfs(struct fs_mount_t *mountp,
static int temp_mount(struct fs_mount_t *mountp)
{
size_t len = strlen(mountp->mnt_point);
if (mountp == NULL) {
return -EINVAL;
}
if (mountp->mnt_point[mountp->mountp_len - 1] != ':') {
if (mountp->mnt_point[len - 1] != ':') {
return -EINVAL;
}
mp[mountp->type] = mountp;