After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 708525 - A "g_file_query_info" on the file path "/sys/kernel/debug/hid" causes occasional Kernel Panic
A "g_file_query_info" on the file path "/sys/kernel/debug/hid" causes occasio...
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal major
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-09-21 11:09 UTC by catapult
Modified: 2017-10-02 12:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GLocalFileInfo: don't content-sniff zero-length files (1.22 KB, patch)
2013-10-01 08:22 UTC, Allison Karlitskaya (desrt)
committed Details | Review
GLocalFileInfo: don't sniff in /dev, /proc, /sys (1.28 KB, patch)
2013-10-01 08:22 UTC, Allison Karlitskaya (desrt)
none Details | Review

Description catapult 2013-09-21 11:09:14 UTC
I use the below piece of code in one of my GTK3 apps to extract content-related information about a file:

(This function takes a Filename and returns the content type of that).
char *
get_content_type (char *filename)
{
  GFile *file = NULL;
  GFileInfo *fileinfo = NULL;
  GError *g_error = NULL;
  char *content_type = NULL, *filename_content_type = NULL;

  file = g_file_new_for_path (filename);

  fileinfo =
    g_file_query_info (file, "standard::content-type",
                       G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, NULL, &g_error);

  if (g_error == NULL)
    {
      content_type =
        (char *) g_file_info_get_attribute_string (fileinfo,
                                                   G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE);
      filename_content_type = strdup (content_type);
    }

  if (fileinfo != NULL)
    g_object_unref (fileinfo);
  if (file != NULL)
    g_object_unref (file);
  if (g_error != NULL)
    g_error_free (g_error);

  return filename_content_type;
}

Usually, when the above function is executed on the directory/files for "/sys/kernel/debug/hid", the function goes into a non-responsive state and hangs the entire app from atleast half a minute to several.
Sometimes however, it completely crashes the OS by triggering a kernel panic!
Using "file_query_info_async" has also not helped, as the crash still happens once in a while.
I have also observed that other Gnome Apps (Eg:gnome-search-tool) that make use of this API, behave quite the same way, too.  

What could be the reason behind the occasional kernel crash?


Below is my Development System Information:
root@:192.168.41.19:~#uname -a
Linux IMGHBD11 3.2.0-45-generic-pae #70-Ubuntu SMP Wed May 29 20:31:05 UTC 2013 i686 i686 i386 GNU/Linux
root@:192.168.41.19:~#lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 12.04.2 LTS
Release:	12.04
Codename:	precise


Thanks,
Catapult.
Comment 1 catapult 2013-09-22 20:11:32 UTC
The Blocking seems to have been caused due to tricky usage of Gdk Threads in my App.
Will reaffirm and Re-open the issue if still relevant.

Changing it to resolved for now.



Catapult.
Comment 2 Allison Karlitskaya (desrt) 2013-09-23 02:40:44 UTC
You should consider filing this as a bug against the kernel.  The kernel should not be panicking as a result of querying things in /sys
Comment 3 catapult 2013-09-24 06:31:34 UTC
I would like to reconsider my previous comment of this being my own design issue, and would like to bring to your attention the reason I think is behind the Application getting hanged:
There are 2 sub-directories in the directory "/sys/kernel/debug/hid":
1. 0003:0461:4D81.0001 
2. 0003:413C:2107.0002
(numbers might be different on different systems)

Each of these sub-directories have 2 files each, "events" and "rdesc".
"events" is the troublesome file that probably gets updated on-the-fly by the kernel to notify interested applications of events happening wrt the corresponding device.

Generally this file is a 0 byte file and grows as more events happen when they do.

There could be several such files scattered over the File System which couldn't be addressed individually.

Now coming to the generic blocking problem:
When we query the content type of files such as these which are 0 bytes in size and yet would block us if attempted to be read, the application trying to do so gets caught up in a blocking I/O Loop.
That might be acceptable for an app which is purely interested in the events corresponding to the particular device, but not for Search-Apps/File Managers which just want to query the file content type and move on to other files.

I have seen this hang happen with "gnome-search-tool", my own application, "grep" and also "cat".
All of these try to open the file in read-only mode and try to sniff the contents and get blocked in the process, as a result of which the entire app hangs until data becomes available.

In the GLib 2.32.3 Source code:
The actual code in gio that performs the query for CONTENT_TYPE is:

gio/glocalfileinfo.c:Line 1251->
###############################################################################
  fd = open (path, O_RDONLY | O_NOATIME);
          if (fd < 0 && errno == EPERM)
#endif
            fd = open (path, O_RDONLY);
  
          if (fd != -1)
            {          
              ssize_t res;
                       
              res = read (fd, sniff_buffer, sniff_length);
              close (fd);
              if (res >= 0)
                {
                  g_free (content_type);
                  content_type = g_content_type_guess (basename, sniff_buffer, res, NULL);
                }
            }
        }   
#endif    
            
      return content_type;
    } 
      
}   
###############################################################################

This "read" call probably hangs the App until data becomes available (which may not be relied upon).

"g_file_query_async" doesn’t help too, as the asynchronous job never completes and has to be cancelled eventually from another thread.

Although this may seem to be a trivial issue, if the size of the files and the blocking status of these on-the-fly files are taken into consideration inside "g_file_query_info" while sniffing data from these files:
a. If file is not empty, try non-blocking I/O or select with a timeout, and see if the function call times out.
b. If the File is empty.
c. If any of the above is true, go by the File extension or File Meta Attributes (obtained through stat/lstat or ACL APIs).

This would really help an app like a search machine/file manager from getting over being blocked on a just single file query, without resorting to multiple threads/complicated IPC.
This is just a suggestion.



Thanks & Regards,
Catapult.
Comment 4 Allison Karlitskaya (desrt) 2013-10-01 08:08:03 UTC
There are three things that I consider to be possible here:

 - don't content-sniff files that stat reports as being 0 bytes in length

 - check the result of statfs and blacklist procfs, sysfs and maybe some
   others from content sniffing

 - do the same thing on the basis of certain path prefixes (/dev, /sys, /proc)
Comment 5 Allison Karlitskaya (desrt) 2013-10-01 08:22:05 UTC
Created attachment 256163 [details] [review]
GLocalFileInfo: don't content-sniff zero-length files

This will prevent attempting to read from some files that appear normal but are
really device-like, such as those in /proc and /sys.

If we can't stat() the file then don't bother attempting to sniff, either.
Comment 6 Allison Karlitskaya (desrt) 2013-10-01 08:22:07 UTC
Created attachment 256164 [details] [review]
GLocalFileInfo: don't sniff in /dev, /proc, /sys

Skip content type detection for files in the above directories in order
to avoid attempting to read from a normal-looking file that is actually
device-like.
Comment 7 Allison Karlitskaya (desrt) 2013-10-01 08:23:06 UTC
There is no statvfs/statfs() call in glocalfileinfo and I really don't want to add one (considering how problematic it is to handle such a call in a portable way), but I am happy with either or both of the above patches (if Alex reviews them).

Either one should solve the issues you reported.
Comment 8 Alexander Larsson 2013-10-01 15:03:25 UTC
Review of attachment 256163 [details] [review]:

In general I like this. There is no need to read the data for empty files for "normal" files that are empty, and lots of reasons not to do it on "non-normal" empty files.

However, the freedesktop mime db has:
  <mime-type type="application/x-zerosize">
    <comment>empty document</comment>

So, not sniffing here is an API change. Maybe we should manually add this?
Comment 9 Alexander Larsson 2013-10-01 15:07:01 UTC
Review of attachment 256164 [details] [review]:

I'm less sure about this. The files in these directories are generally without extensions, so the non-sniffed name is useless.
For /dev its not a great problem, most files there are device nodes or directories, so the stat data is enough there. However, for /proc and /sys, this means that we just return "unknown" for basically everything. Not sure if that is right...
Comment 10 Allison Karlitskaya (desrt) 2013-10-01 19:26:23 UTC
What sort of useful user-interesting content would you expect to find in /sys or /proc, anyway?

btw:

[desrt@moonpix ~]$ ls a
ls: cannot access a: No such file or directory
[desrt@moonpix ~]$ touch a
[desrt@moonpix ~]$ gvfs-info a
...
  standard::icon: text-plain, text-x-generic
  standard::content-type: text/plain
  standard::fast-content-type: application/octet-stream


I'm not sure what circumstances application/x-zerosize is returned under, if not for zero-sized files with no extension...
Comment 11 Alexander Larsson 2013-10-02 07:42:51 UTC
I don't really expect user-interesting content in terms of e.g. browsing /sys with nautilus. GIO is a lower level API though, which may be used for other stuff. Its true that such use rarely rely on content types though.
However, if its not ever used, is it a problem to support it? I guess it may be accidentally invoked via recursive operations, and trigger a bunch of kernel code that may be questionable (and slow) similar to the size zero issue.
So, I'm not against it, I just wonder how important it is.

Also, you can move the strcmps to _g_local_file_info_get_parent_info, so that they don't have to happen on each stat() in a readdir() operation.


As for application/x-zerosize, yeah, that doesn't seem to work. There is a bug:
https://bugs.freedesktop.org/show_bug.cgi?id=55120 which says its broken without caches, but it seems broken with caches here too...

Here is the commit by David that added it:
http://cgit.freedesktop.org/xdg/xdgmime/commit/?id=5181175d5fdaa3832b0fd094cda0120b1fe92af6
I'm not sure what the reason for it was, but we should maybe ask him.
Comment 12 Matthias Clasen 2015-08-21 05:01:24 UTC
Comment on attachment 256163 [details] [review]
GLocalFileInfo: don't content-sniff zero-length files

Pushed the first patch, but not the second.
Attachment 256163 [details] pushed as b6fc1df - GLocalFileInfo: don't content-sniff zero-length files
Comment 13 Philip Withnall 2017-10-02 12:38:02 UTC
I think this can be resolved. The original bug has been fixed, and I agree that since GIO can be used for low-level things, it should not arbitrarily fail to content-sniff /sys, /dev and /proc.