xorl %eax, %eax

Linux dell_rbu.c bugs

leave a comment »

This is a low impact vulnerability found by Linus Torvalds on 17 January 2009. But first of all, a few words about the DELL RBU (Remote BIOS Update). Dell RBU is a device driver used to update BIOS firmware on DELL servers. After loading the driver the following special files are being created:

/sys/class/firmware/dell_rbu/loading
/sys/class/firmware/dell_rbu/data
/sys/devices/platform/dell_rbu/image_type
/sys/devices/platform/dell_rbu/data
/sys/devices/platform/dell_rbu/packet_size

Users can utilize these files to manage the update process, for example, if you want to change the default (which is monolithic) update image type to.. let’s say, packet you can do it like:

echo "packet" > /sys/devices/platform/dell_rbu/image_type

Now, let’s move to the vulnerability. This bug affects Linux kernel up to 2.6.28.2 release and can be found at drivers/firmware/dell_rbu.c. Here is the vulnerable function as it is on 2.6.28 release of the Linux kernel.

644 static ssize_t read_rbu_packet_size(struct kobject *kobj,
645                                    struct bin_attribute *bin_attr,
646                                    char *buffer, loff_t pos, size_t count)
647 {

This function is used to parse the input of the sysfs packet_size file of the above sysfs files created by the driver. Let’s examine the function more extensively. It takes 5 arguments which are:

struct kobject *kobj            -  A structure used to describe a sysfs kernel object
struct bin_attribute *bin_attr  -  Available attributes on the sysfs file
char *buffer                    -  A temporary buffer used internally
loff_t pos                      -  An offset variable for usage in the sysfs file
size_t count                    -  A counter for size limit of the user controlled string.

It is a simple function, here is what it does:

648        int size = 0;
649        if (!pos) {
650                spin_lock(&rbu_data.lock);
651                size = sprintf(buffer, "%lu\n", rbu_data.packetsize);
652                spin_unlock(&rbu_data.lock);
653        }
654        return size;
655 }

If the offset position is NULL then it just returns 0. In any other case, it creates a spinlock on the file to avoid race conditions and then, using sprintf() it retrieves the packet size from the requested attribute of the sysfs file. This structure, is a static buffer that is used to hold information regarding the attributes. It has the following members:

55 static struct _rbu_data {
56        void *image_update_buffer;
57        unsigned long image_update_buffer_size;
58        unsigned long bios_image_size;
59        int image_update_ordernum;
60        int dma_alloc;
61        spinlock_t lock;
62        unsigned long packet_read_count;
63        unsigned long num_packets;
64        unsigned long packetsize;
65        unsigned long imagesize;
66        int entry_created;
67 } rbu_data;

The problem of the above code is located at line 651. This is a critical vulnerability since attacker can change /sys/devices/platform/dell_rbu/packet_size with unprivileged users by default. If a user is able to give values less than, or equal to zero he might be able to cause a segmentation violation. This is why the above code was patched like this:

     spin_lock(&rbu_data.lock);
-    size = sprintf(buffer, "%lu\n", rbu_data.packetsize);
+    size = scnprintf(buffer, count, "%lu\n", rbu_data.packetsize);
     spin_unlock(&rbu_data.lock);

In the given patch, the count parameter is taken from the arguments of the function. In addition to this vulnerability there was another one more important since it was parsing user controlled string values. This was the image_type which was briefly discussed earlier. Here is the buggy function:

573 static ssize_t read_rbu_image_type(struct kobject *kobj,
574                                   struct bin_attribute *bin_attr,
575                                   char *buffer, loff_t pos, size_t count)
576 {
577        int size = 0;
578        if (!pos)
579                size = sprintf(buffer, "%s\n", image_type);
580        return size;
581 }

This is clearly a BAD idea! User can insert arbitrary strings simple by doing an echo to the appropriate sysfs file! To fix this the patch was this:

     if (!pos)
-        size = sprintf(buffer, "%s\n", image_type);
+        size = scnprintf(buffer, count, "%s\n", image_type);
     return size;

The advantage of using scnprintf(9) on the first vulnerability is that the latter library routine returns 0 if the size was less than, or equal to zero which is something that sprintf(3) doesn’t do. On the second function the advantages are obvious, first the size limit and second the return value. This is probably the easiest to trigger kernel bug I’ve discussed here so far.

Written by xorl

January 26, 2009 at 17:06

Posted in bugs, linux

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s