--- Begin Message ---
- To: Debian Bug Tracking System <submit@bugs.debian.org>
- Subject: mtpfs has some issues with wrong indexes
- From: Tobias Schlemmer <keinstein@users.berlios.de>
- Date: Sun, 06 Oct 2013 00:07:29 +0200
- Message-id: <20131005220729.5765.73564.reportbug@tobias4.schlemmersoft.de>
Package: mtpfs
Version: 1.1-4.1
Severity: normal
Tags: upstream patch
I had some problems with my Samsung YP-T10. Tracking down the problem it turned
out that some index variables are not checked correctly. In particular this
involves some off by 1 string calculations and the return value of find_storage
which may return -1 while this case is not handled at all.
Additionally the package is not usable at least on my system. Trying to move
some files on the mentioned device caused data loss as mv did not get an error,
even though the data was not transferred to the device.
I'll attach a patch that solves some of the issues. I added checks for wrong
index entries where I found them and reorganized some code in a way such that
certan errors should not happen anymore.
In summary: I wonder why this package worked at all under certain
circumstances. Even on my system.
-- System Information:
Debian Release: jessie/sid
APT prefers testing-proposed-updates
APT policy: (500, 'testing-proposed-updates'), (500, 'oldstable-updates'), (500, 'unstable'), (500, 'testing'), (500, 'oldstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 3.10-2-amd64 (SMP w/6 CPU cores)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages mtpfs depends on:
ii fuse 2.9.2-4
ii libc6 2.17-92+b1
ii libfuse2 2.9.2-4
ii libglib2.0-0 2.36.4-1
ii libid3tag0 0.15.1b-10
ii libmad0 0.15.1b-8
ii libmtp9 1.1.6-20-g1b9f164-1
ii libusb-1.0-0 2:1.0.17-1+b1
ii zlib1g 1:1.2.8.dfsg-1
mtpfs recommends no packages.
mtpfs suggests no packages.
-- no debconf information
diff -ur mtpfs-1.1.debian/mtpfs.c mtpfs-1.1/mtpfs.c
--- mtpfs-1.1.debian/mtpfs.c 2013-10-05 17:12:45.000000000 +0200
+++ mtpfs-1.1/mtpfs.c 2013-10-05 23:16:06.000000000 +0200
@@ -7,6 +7,7 @@
*/
#include <mtpfs.h>
+#include <stdlib.h>
#if DEBUG
#define STRINGIFY(x) #x
@@ -86,7 +87,7 @@
parent_found = TRUE;
} else {
int i;
- for (i=0;i<4;i++) {
+ for (i=0;i<MAX_STORAGE;i++) {
if (storageArea[i].folders!=NULL) {
if (LIBMTP_Find_Folder (storageArea[i].folders, item->parent_id) != NULL) {
parent_found = FALSE;
@@ -111,7 +112,7 @@
check_folders ()
{
int i;
- for (i=0;i<4;i++) {
+ for (i=0;i<MAX_STORAGE;i++) {
if (storageArea[i].folders_changed) {
DBG("Refreshing Folderlist %d-%s", i,storageArea[i].storage->StorageDescription);
LIBMTP_folder_t *newfolders = NULL;
@@ -304,8 +305,9 @@
find_storage(const gchar * path)
{
int i;
+ if (!path || path[0] != '/') return -1;
DBG("find_storage:%s",path);
- for (i=0;i<4;i++) {
+ for (i=0;i<MAX_STORAGE;i++) {
if (storageArea[i].storage != NULL) {
int maxlen = strlen(storageArea[i].storage->StorageDescription);
if (strlen(path+1) < maxlen) maxlen = strlen(path+1);
@@ -342,7 +344,7 @@
gchar *current;
current = g_strconcat(parent, "/", folderlist->name,NULL);
- LIBMTP_devicestorage_t *storage;
+ /* LIBMTP_devicestorage_t *storage; */
DBG("compare %s,%s",mypath, current);
if (g_ascii_strcasecmp (mypath, current) == 0) {
@@ -414,12 +416,19 @@
LIBMTP_folder_t *folder;
gchar **fields;
gchar *directory;
- directory = (gchar *) g_malloc (strlen (path));
+ directory = (gchar *) g_malloc (strlen (path)+1);
directory = strcpy (directory, "");
fields = g_strsplit (path, "/", -1);
res = -ENOENT;
int storageid;
storageid = find_storage(path);
+ if (storageid < 0) {
+ g_free (directory);
+ g_strfreev (fields);
+ DBG("parse_path exiting:%s - %d",path,res);
+ return res;
+ }
+
for (i = 0; fields[i] != NULL; i++) {
if (strlen (fields[i]) > 0) {
if (fields[i + 1] != NULL) {
@@ -486,13 +495,21 @@
gchar *filename = g_strdup("");
gchar **fields;
gchar *directory;
- directory = (gchar *) g_malloc (strlen (path));
+ directory = (gchar *) g_malloc (strlen (path)+1);
directory = strcpy (directory, "/");
fields = g_strsplit (path, "/", -1);
- int i;
+ int i=0;
int parent_id = 0;
int storageid;
- storageid = find_storage(fields[0]);
+ if (!(*fields[0])) i++;
+ storageid = find_storage(path);
+
+ if (storageid < 0) {
+ DBG("release: unable to save file %s: no such storage", path);
+ close (fi->fh);
+ return_unlock(-ENOENT);
+ }
+
for (i = 0; fields[i] != NULL; i++) {
if (strlen (fields[i]) > 0) {
if (fields[i + 1] == NULL) {
@@ -628,7 +645,7 @@
enter_lock("destroy");
if (files) free_files(files);
int i;
- for (i=0;i<4;i++) {
+ for (i=0;i<MAX_STORAGE;i++) {
if (storageArea[i].folders) LIBMTP_destroy_folder_t(storageArea[i].folders);
}
if (playlists) free_playlists(playlists);
@@ -660,7 +677,7 @@
st.st_nlink = 2;
st.st_ino = storage->id;
st.st_mode = S_IFREG | 0555;
- gchar *name;
+ /* gchar *name; */
filler (buf, storage->StorageDescription, &st, 0);
}
return_unlock(0);
@@ -710,9 +727,13 @@
}
// Get storage area
- int i;
+ /* int i; */
int storageid = -1;
storageid=find_storage(path);
+ if (storageid < 0) {
+ return_unlock(-ENOENT);
+ }
+
// Get folder listing.
int folder_id = 0;
if (strcmp (path, "/") != 0) {
@@ -749,7 +770,7 @@
LIBMTP_destroy_folder_t (folder);
DBG("Checking files");
// Find files
- LIBMTP_file_t *file, *tmp;
+ LIBMTP_file_t *file /* , *tmp */;
check_files();
file = files;
while (file != NULL) {
@@ -762,7 +783,7 @@
if (filler (buf, (file->filename == NULL ? "<mtpfs null>" : file->filename), &st, 0))
break;
}
- tmp = file;
+ /* tmp = file; */
file = file->next;
}
DBG("readdir exit");
@@ -773,7 +794,7 @@
mtpfs_getattr_real (const gchar * path, struct stat *stbuf)
{
int ret = 0;
- if (path==NULL) return_unlock(-ENOENT);
+ if (path==NULL || path[0]==0) return_unlock(-ENOENT);
memset (stbuf, 0, sizeof (struct stat));
// Set uid/gid of file
@@ -801,17 +822,12 @@
}
// Special case directory 'Playlists', 'lost+found'
- // Special case root directory items
- if (g_strrstr(path+1,"/") == NULL) {
- stbuf->st_mode = S_IFDIR | 0777;
- stbuf->st_nlink = 2;
- return_unlock(0);
- }
-
- int storageid;
- storageid=find_storage(path);
-
if (g_ascii_strncasecmp (path, "/Playlists",10) == 0) {
+ if (!path+10 || (path+10 == '/' && !(path+11))) {
+ stbuf->st_mode = S_IFDIR | 0777;
+ stbuf->st_nlink = 2;
+ return_unlock(0);
+ }
LIBMTP_playlist_t *playlist;
check_playlists();
playlist=playlists;
@@ -830,7 +846,11 @@
filesize = filesize + strlen(file->filename) + 2;
while (parent_id != 0) {
check_folders();
- folder = LIBMTP_Find_Folder(storageArea[i].folders,parent_id);
+ int j = 0;
+ do {
+ folder = LIBMTP_Find_Folder(storageArea[j].folders,parent_id);
+ j++;
+ } while (j < MAX_STORAGE && !folder);
parent_id = folder->parent_id;
filesize = filesize + strlen(folder->name) + 1;
}
@@ -848,24 +868,42 @@
}
if (strncasecmp (path, "/lost+found",11) == 0) {
- GSList *item;
- int item_id = parse_path (path);
- for (item = lostfiles; item != NULL; item = g_slist_next (item)) {
- LIBMTP_file_t *file = (LIBMTP_file_t *) item->data;
-
- if (item_id == file->item_id) {
- stbuf->st_ino = item_id;
- stbuf->st_size = file->filesize;
- stbuf->st_blocks = (file->filesize / 512) +
- (file->filesize % 512 > 0 ? 1 : 0);
- stbuf->st_nlink = 1;
- stbuf->st_mode = S_IFREG | 0777;
+ GSList *item;
+ int item_id = parse_path (path);
+ if (!path+11 || (path+11 == '/' && !(path+12))) {
+ stbuf->st_mode = S_IFDIR | 0777;
+ stbuf->st_nlink = 2;
+ return_unlock(0);
+ }
+
+ for (item = lostfiles; item != NULL; item = g_slist_next (item)) {
+ LIBMTP_file_t *file = (LIBMTP_file_t *) item->data;
+ if (item_id == file->item_id) {
+ stbuf->st_ino = item_id;
+ stbuf->st_size = file->filesize;
+ stbuf->st_blocks = (file->filesize / 512) +
+ (file->filesize % 512 > 0 ? 1 : 0);
+ stbuf->st_nlink = 1;
+ stbuf->st_mode = S_IFREG | 0777;
stbuf->st_mtime = file->modificationdate;
- return_unlock(0);
- }
- }
+ return_unlock(0);
+ }
+ }
- return_unlock(-ENOENT);
+ return_unlock(-ENOENT);
+ }
+
+ int storageid;
+ storageid=find_storage(path);
+ if (storageid < 0) {
+ return_unlock(-ENOENT);
+ }
+
+ // Special case root directory items
+ if (g_strrstr(path+1,"/") == NULL) {
+ stbuf->st_mode = S_IFDIR | 0777;
+ stbuf->st_nlink = 2;
+ return_unlock(0);
}
int item_id = -1;
@@ -879,6 +917,9 @@
} else {
// Must be a file
item_id = parse_path (path);
+ if (item_id < 0) {
+ return -ENOENT;
+ }
LIBMTP_file_t *file;
DBG("id:path=%d:%s", item_id, path);
check_files();
@@ -948,8 +989,10 @@
DBG("rdwrite");
}
- int storageid;
- storageid=find_storage(path);
+ /*
+ int storageid;
+ storageid=find_storage(path);
+ */
FILE *filetmp = tmpfile ();
int tmpfile = fileno (filetmp);
if (tmpfile != -1) {
@@ -980,12 +1023,20 @@
path = strcpy(path,"/");
int parent_id = file->parent_id;
while (parent_id != 0) {
+ int j;
check_folders();
- folder = LIBMTP_Find_Folder(storageArea[storageid].folders,parent_id);
- path = strcat(path,folder->name);
- path = strcat(path,"/");
- parent_id = folder->parent_id;
- }
+ for (j = 0 ; j < MAX_STORAGE && !folder ; j++) {
+ folder = LIBMTP_Find_Folder(storageArea[j].folders,parent_id);
+ }
+ if (folder) {
+ path = strcat(path,folder->name);
+ path = strcat(path,"/");
+ parent_id = folder->parent_id;
+ } else {
+ path = strcat(path,"<unknown mtp folder>/");
+ parent_id = 0;
+ }
+ }
path = strcat (path,file->filename);
fprintf (filetmp,"%s\n",path);
DBG("%s\n",path);
@@ -1082,7 +1133,9 @@
item = g_slist_find_custom (myfiles, path, (GCompareFunc) strcmp);
int item_id = parse_path (path);
int storageid = find_storage(path);
- if ((item == NULL) && (item_id < 0)) {
+ if (storageid < 0) {
+ ret = -ENOENT; // \todo: use another return value?
+ } else if ((item == NULL) && (item_id < 0)) {
// Split path and find parent_id
gchar *filename = g_strdup("");
gchar **fields;
@@ -1147,6 +1200,8 @@
return_unlock(0);
}
int storageid=find_storage(path);
+ if (storageid < 0)
+ return_unlock(-ENOENT);
folder_id = lookup_folder_id (storageArea[storageid].folders, (gchar *) path, NULL);
if (folder_id < 0)
return_unlock(-ENOENT);
@@ -1200,9 +1255,9 @@
int
mtpfs_rename (const char *oldname, const char *newname)
{
- enter_lock("rename '%s' to '%s'", oldname, newname);
+ enter_lock("rename '%s' to '%s'", oldname, newname);
- int folder_id = -1, parent_id;
+ int folder_id = -1 /* , parent_id */;
int folder_empty = 1;
int ret = -ENOTEMPTY;
LIBMTP_folder_t *folder;
@@ -1210,6 +1265,9 @@
int storageid_old=find_storage(oldname);
int storageid_new=find_storage(newname);
+ if (storageid_old < 0 || storageid_new < 0) {
+ return_unlock(-ENOENT);
+ }
if (strcmp (oldname, "/") != 0) {
folder_id = lookup_folder_id (storageArea[storageid_old].folders, (gchar *) oldname, NULL);
}
@@ -1223,7 +1281,7 @@
if (folder == NULL)
return_unlock(-ENOENT);
- parent_id = folder->parent_id;
+ /* parent_id = folder->parent_id; */
folder = folder->child;
/* Check if empty folder */
@@ -1285,7 +1343,7 @@
void *
mtpfs_init ()
{
- LIBMTP_devicestorage_t *storage;
+ /* LIBMTP_devicestorage_t *storage; */
DBG("mtpfs_init");
files_changed=TRUE;
playlists_changed=TRUE;
@@ -1293,14 +1351,24 @@
return 0;
}
+
+/*
int
mtpfs_blank()
{
// Do nothing
+ return 0;
+}
+*/
+int
+mtpfs_chmod (const char * path, mode_t mode) {
+ // Do nothing
+ return 0;
}
+
static struct fuse_operations mtpfs_oper = {
- .chmod = mtpfs_blank,
+ .chmod = mtpfs_chmod,
.release = mtpfs_release,
.readdir = mtpfs_readdir,
.getattr = mtpfs_getattr,
@@ -1325,9 +1393,9 @@
LIBMTP_raw_device_t * rawdevices;
int numrawdevices;
LIBMTP_error_number_t err;
- int i;
+ int i = 0;
- int opt;
+ /* int opt; */
extern int optind;
extern char *optarg;
@@ -1419,7 +1487,7 @@
/* Check if multiple storage areas */
LIBMTP_devicestorage_t *storage;
i=0;
- for (storage = device->storage; storage != 0; storage = storage->next) {
+ for (storage = device->storage; storage != 0 && i < MAX_STORAGE; storage = storage->next) {
storageArea[i].storage=storage;
storageArea[i].folders=NULL;
storageArea[i].folders_changed=TRUE;
@@ -1427,6 +1495,11 @@
i++;
}
+ if (storage) {
+ printf("Warning: the device has more than %d storages\n",MAX_STORAGE);
+ printf(" ftpfs will show only the first %d of them",MAX_STORAGE);
+ }
+
DBG("Start fuse");
fuse_stat=fuse_main (argc, argv, &mtpfs_oper);
Nur in mtpfs-1.1: mtpfs.c~.
diff -ur mtpfs-1.1.debian/mtpfs.h mtpfs-1.1/mtpfs.h
--- mtpfs-1.1.debian/mtpfs.h 2013-10-05 17:12:45.000000000 +0200
+++ mtpfs-1.1/mtpfs.h 2013-10-05 21:48:26.000000000 +0200
@@ -50,7 +50,7 @@
/* fuse functions */
static void * mtpfs_init (void);
-static int mtpfs_blank ();
+//static int mtpfs_blank ();
static int mtpfs_release (const char *path, struct fuse_file_info *fi);
void mtpfs_destroy ();
static int mtpfs_readdir (const gchar * path, void *buf, fuse_fill_dir_t filler, off_t offset, struct fuse_file_info *fi);
@@ -64,9 +64,10 @@
static int mtpfs_rmdir (const char *path);
static int mtpfs_statfs (const char *path, struct statfs *stbuf);
int calc_length(int f);
+#define MAX_STORAGE 4
static LIBMTP_mtpdevice_t *device;
-static StorageArea storageArea[4];
+static StorageArea storageArea[MAX_STORAGE];
static LIBMTP_file_t *files = NULL;
static gboolean files_changed = TRUE;
static GSList *lostfiles = NULL;
Nur in mtpfs-1.1: mtpfs.h~.
Binärdateien mtpfs-1.1.debian/mtpfs-id3read.o und mtpfs-1.1/mtpfs-id3read.o sind verschieden.
Binärdateien mtpfs-1.1.debian/mtpfs-mtpfs.o und mtpfs-1.1/mtpfs-mtpfs.o sind verschieden.
--- End Message ---