From 6e43fb8c0221d1f4759e98bb5bef5c8c687cb6d7 Mon Sep 17 00:00:00 2001 From: Ivan Kadochnikov Date: Thu, 4 May 2017 15:32:00 +0200 Subject: [PATCH 1/4] xrootdfs: check file descriptor before using it in xrootdfs wcache If file open failed with ENOSYS (it can if the server responded with kXR_Unsupported), then file descriptor is not initialized. As a result any attempt to write to the file gives a segfault. This check gives a EBADF instead. --- src/XrdFfs/XrdFfsWcache.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/XrdFfs/XrdFfsWcache.cc b/src/XrdFfs/XrdFfsWcache.cc index 855f6626fc6..54f7590fa15 100644 --- a/src/XrdFfs/XrdFfsWcache.cc +++ b/src/XrdFfs/XrdFfsWcache.cc @@ -161,6 +161,11 @@ ssize_t XrdFfsWcache_pwrite(int fd, char *buf, size_t len, off_t offset) ssize_t rc; char *bufptr; fd -= XrdFfsPosix_baseFD; + if (fd < 0) + { + errno = EBADF; + return -1; + } /* do not use caching under these cases */ if (len > XrdFfsWcacheBufsize/2 || fd >= XrdFfsWcacheNFILES) From f2889a5020e8e065350cae77bba745a794bc3498 Mon Sep 17 00:00:00 2001 From: Ivan Kadochnikov Date: Thu, 4 May 2017 15:32:01 +0200 Subject: [PATCH 2/4] xrootdfs: more error checks when creating write cache Return errors to the caller instead of ignoring them. --- src/XrdFfs/XrdFfsWcache.cc | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/XrdFfs/XrdFfsWcache.cc b/src/XrdFfs/XrdFfsWcache.cc index 54f7590fa15..c71e46d8a99 100644 --- a/src/XrdFfs/XrdFfsWcache.cc +++ b/src/XrdFfs/XrdFfsWcache.cc @@ -102,7 +102,14 @@ void XrdFfsWcache_init(int basefd, int maxfd) } } -int XrdFfsWcache_create(int fd) +int XrdFfsWcache_create(int fd) +/* Create a write cache buffer for a given file descriptor + * + * fd: file descriptor + * + * returns: 1 - ok + * 0 - error, error code in errno + */ { XrdFfsWcache_destroy(fd); fd -= XrdFfsPosix_baseFD; @@ -115,8 +122,9 @@ int XrdFfsWcache_create(int fd) XrdFfsWcacheFbufs[fd].mlock = (pthread_mutex_t*)malloc(sizeof(pthread_mutex_t)); if (XrdFfsWcacheFbufs[fd].mlock == NULL) return 0; - else - pthread_mutex_init(XrdFfsWcacheFbufs[fd].mlock, NULL); + errno = pthread_mutex_init(XrdFfsWcacheFbufs[fd].mlock, NULL); + if (errno) + return 0; return 1; } From 6fcefebcdb5a1f789299c6050b5daf5169864fb6 Mon Sep 17 00:00:00 2001 From: Ivan Kadochnikov Date: Fri, 5 May 2017 15:36:29 +0200 Subject: [PATCH 3/4] xrootdfs: avoid using literal 1024, replace with MAXROOTURLLEN Just a little clean-up to use the constant everywhere, instead of only sometimes. --- src/XrdFfs/XrdFfsMisc.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/XrdFfs/XrdFfsMisc.cc b/src/XrdFfs/XrdFfsMisc.cc index 2f16362c0b0..248e1495339 100644 --- a/src/XrdFfs/XrdFfsMisc.cc +++ b/src/XrdFfs/XrdFfsMisc.cc @@ -119,7 +119,7 @@ int XrdFfsMisc_get_all_urls_real(const char *oldurl, char **newurls, const int n used a cache to reduce unnecessary queries to the redirector */ -char XrdFfsMiscCururl[1024] = ""; +char XrdFfsMiscCururl[MAXROOTURLLEN] = ""; char *XrdFfsMiscUrlcache[XrdFfs_MAX_NUM_NODES]; int XrdFfsMiscNcachedurls = 0; time_t XrdFfsMiscUrlcachetime = 0; @@ -211,7 +211,7 @@ int XrdFfsMisc_get_list_of_data_servers(char* list) const char *hName, *hNend, *hPort, *hPend; char *url, *rc, *hostip, hsep; - rc = (char*)malloc(sizeof(char) * XrdFfs_MAX_NUM_NODES * 1024); + rc = (char*)malloc(sizeof(char) * XrdFfs_MAX_NUM_NODES * MAXROOTURLLEN); rc[0] = '\0'; pthread_mutex_lock(&XrdFfsMiscUrlcache_mutex); for (i = 0; i < XrdFfsMiscNcachedurls; i++) @@ -418,7 +418,7 @@ void XrdFfsMisc_xrd_secsss_register(uid_t user_uid, gid_t user_gid, int *id) void XrdFfsMisc_xrd_secsss_editurl(char *url, uid_t user_uid, int *id) { - char user_num[9], nurl[1024], *tmp; + char user_num[9], nurl[MAXROOTURLLEN], *tmp; // Xrootd proxy also use some of the stat()/unlink(), etc funcitons here, without user "sss". It use the default // connection login name. Don't change this behavior. (so for proxy, use no "sss", and always have *id = 0 @@ -431,11 +431,11 @@ void XrdFfsMisc_xrd_secsss_editurl(char *url, uid_t user_uid, int *id) else user_num[strlen(user_num)] = *id + 48; nurl[0] = '\0'; - strcat(nurl, "root://"); - strcat(nurl, user_num); - strcat(nurl, "@"); - strcat(nurl, &(url[7])); - strcpy(url, nurl); + strncat(nurl, "root://", 7); + strncat(nurl, user_num, 9); + strncat(nurl, "@", 1); + strncat(nurl, &(url[7]), MAXROOTURLLEN-17); + strncpy(url, nurl, MAXROOTURLLEN); } } From e7d027e2933467e781238f069106b9a774baaaa3 Mon Sep 17 00:00:00 2001 From: Ivan Kadochnikov Date: Fri, 5 May 2017 15:59:46 +0200 Subject: [PATCH 4/4] xrootdfs: actually check wcache creation error on open Report the error to caller instead of ignoring it. --- src/XrdFfs/XrdFfsXrootdfs.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/XrdFfs/XrdFfsXrootdfs.cc b/src/XrdFfs/XrdFfsXrootdfs.cc index 64e83aa733b..77146dd6bfe 100644 --- a/src/XrdFfs/XrdFfsXrootdfs.cc +++ b/src/XrdFfs/XrdFfsXrootdfs.cc @@ -715,21 +715,30 @@ static int xrootdfs_utimens(const char *path, const struct timespec ts[2]) } static int xrootdfs_open(const char *path, struct fuse_file_info *fi) +/* + * path: path to file + * fi: file info, return file descriptor in fi->fh + * + * returns 0 on success and -errno on error + */ { - int res, lid = 1; + int fd, lid = 1; char rootpath[MAXROOTURLLEN]=""; strncat(rootpath,xrootdfs.rdr, MAXROOTURLLEN - strlen(rootpath) -1); strncat(rootpath,path, MAXROOTURLLEN - strlen(rootpath) -1); XrdFfsMisc_xrd_secsss_register(fuse_get_context()->uid, fuse_get_context()->gid, &lid); XrdFfsMisc_xrd_secsss_editurl(rootpath, fuse_get_context()->uid, &lid); - res = XrdFfsPosix_open(rootpath, fi->flags, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH); - if (res == -1) + fd = XrdFfsPosix_open(rootpath, fi->flags, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH); + if (fd == -1) return -errno; - fi->fh = res; - XrdFfsWcache_create(fi->fh); - return 0; + fi->fh = fd; + // be careful, 0 means error for this function + if (XrdFfsWcache_create(fi->fh)) + return 0; + else + return -errno; } static int xrootdfs_read(const char *path, char *buf, size_t size, off_t offset,