2

I'm seeing an oddity I don't understand in the output of my code. I have a structure defined in a header file. I populate a structure in user space then send it via ioctl to a kernel module. The kernel module should copy it from the user then report the values stored by the user.

The structure is defined as:

typedef struct Command_par {
    int cmd;            /**< special driver command */
    int target;         /**< special configuration target */
    unsigned long val1;     /**< 1. parameter for the target */
    unsigned long val2;     /**< 2. parameter for the target */
    int error;          /**< return value */
    unsigned long retval;   /**< return value */
} Command_par_t ;

The user space code is just a quick test program:

#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <fcntl.h>
#include <sys/stat.h>
#include "can4linux.h"  //can4linux is the standard can driver that comes
                        //with the uCLinux kernel (where this code was originally
                        //running before this port to a desktop machine

#define CAN_COMMAND 0
void main()
{
  long ret;
  Command_par_t cmd;
  int fd;
  //set and open the file descriptor to the device

  cmd.cmd = 2;
  cmd.target = 9; 
  cmd.val1 = 8;
  cmd.val2 = 7;
  cmd.error = 6;
  cmd.retval = 5;
  ret = ioctl(fd, CAN_COMMAND, &cmd);

The kernel module getting opened/sent data via ioctl:

long can_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
  Command_par_t *myptr;
  myptr = (void *)kmalloc(sizeof(Command_par_t)+1,GFP_KERNEL );

  copy_from_user((void *)myptr, (void *)arg, sizeof(Command_par_t));

  printk("cmd = %d, target = %d, val1 = %d, val2 = %d, error = %d, retval = %d\n", 
         myptr->cmd, myptr->target, myptr->val1, myptr->val2, myptr->error, myptr->retval);

Here's where the possible IOCTL commands are defined in the can4linux.h header:

---------- IOCTL requests */

#define COMMAND      0  /**< IOCTL command request */
#define CONFIG       1  /**< IOCTL configuration request */
#define SEND         2  /**< IOCTL request */
#define RECEIVE      3  /**< IOCTL request */
#define CONFIGURERTR     4  /**< IOCTL request */

So obviously this is just test code, I'm trying to debug a bigger problem, but right now even this isn't working correctly... The output I'm getting is:

[217088.860190] cmd = 2, target = 1, val1 = 3, val2 = 134514688, error = 134514480, retval = 0

The cmd was set correctly, after that everything else is skewed... Looks like it only passed an int and then I'm accessing memory outside of my struct? Anyone see what I'm doing wrong here?

Mike
  • 47,263
  • 29
  • 113
  • 177
  • Is it a 32 or 64 bit kernel? This might affect the size of `unsigned long`, and the proper format string for it. `"%lu"`? – Bo Persson Sep 10 '12 at 15:26
  • It's a 32-bit kernel. (technically a 32-bit OpenSUSE 12.1 OS running within a VirtualBox on a 64-bit Windows 7 machine) – Mike Sep 10 '12 at 15:29
  • Just to confirm: `uname -a` `Linux linux-4puc.site 3.1.0-1.2-desktop #1 SMP PREEMPT Thu Nov 3 14:45:45 UTC 2011 (187dde0) i686 i686 i386 GNU/Linux` – Mike Sep 10 '12 at 15:29
  • @BoPersson - Now that I'm thinking about it... that can't really be the problem can it? if the unsigned long was messing up my calls wouldn't we at least see cmd and target come across correctly? – Mike Sep 10 '12 at 15:34
  • No, that was just one possibility. That's why I wrote it as a comment, and not an answer. ;-) – Bo Persson Sep 10 '12 at 15:40
  • 2
    Can you show the definition of the ioctl number? Also the includes you're using in userspace? – jleahy Sep 10 '12 at 15:44
  • @jleahy - I made the updates you requested... and I think I see what you're driving at. You're thinking the IOCTL defines might have been "unique" enough for my old system but on the new system they're too generic and need to be redefined using the more standard `_IOW()`? – Mike Sep 10 '12 at 16:02
  • Uniqueness isn't a problem, but the ioctl numbers are 'magic' values, they have the read/writeness encoded into them, along with the size of the data that gets copied across. I'm not sure if it's used except for strace, but it might be worth using _IOW macro just to be safe. Take a look at http://lwn.net/Articles/48354/. The other issue is to make sure you're including ioctl.h from the right place (sys/ioctl.h in userspace, not linux/ioctl.h). – jleahy Sep 10 '12 at 16:14
  • Don't use 0 as ioctl number. Use `_IO` macro instead. Use __attribute__((packed)) with struct also. – Ilya Matveychikov Sep 10 '12 at 17:14
  • @IlyaMatveychikov - If I'm not concerned with unused space, why use the __attribute__((packed))? – Mike Sep 10 '12 at 17:26
  • @Mike: Kernel and userspace compiler options may be different. So you'll never know exactly how structures are packed. – Ilya Matveychikov Sep 11 '12 at 13:21
  • @Mike: Look at this question also, http://stackoverflow.com/questions/10071296/ioctl-is-not-called-if-cmd-2/10071360#10071360 – Ilya Matveychikov Sep 11 '12 at 13:22

1 Answers1

1

The problem was how the ioctl number was being defined. This was a port from a uCLinux (2.4) running on some embedded HW that we have to a OpenSUSE distribution (3.1). I guess what was happening was on the smaller embedded platform we were getting lucky and the ioctl number defined as simple integers:

#define COMMAND      0  /**< IOCTL command request */ 
#define CONFIG       1  /**< IOCTL configuration request */ 
#define SEND         2  /**< IOCTL request */ 
#define RECEIVE      3  /**< IOCTL request */ 
#define CONFIGURERTR     4  /**< IOCTL request */ 

Were sufficiently unique enough to not cause problems. Now that I'm trying to recompile the code on a full linux distribution there's too much other stuff going on in the system and our commands were getting trampled on. Now redefining the ioctl numbers as such:

#define GC_CAN_IOC_MAGIC  'z'       //!< Magic number - I guess this magic number can be chosen freely
#define GC_CAN_IOC_WRITE_COMMAND        _IOW(GC_CAN_IOC_MAGIC,  0, Command_par_t)

The values passed from user space are correctly recorded:

[220546.535115] cmd = 2, target = 9, val1 = 8, val2 = 7, error = 6, retval = 5

If anyone can give me a better answer than "trampled on" I'd be happy to have a clearer understanding of this problem. Also if I'm setting up the IOCTL numbers wrongly still, please someone correct me.

Mike
  • 47,263
  • 29
  • 113
  • 177