On Mon, Apr 08, 2002, Dominik Kuhlen <[EMAIL PROTECTED]> wrote:
> Here it is : my full code.
> I've done some improvements and now it seems to
> work. But only the first time. The second try kills the kernel :-(
> I think I'll change to libusb (hope its much safer).
If you can kill the kernel with libusb, I'd be surprised. usbdevfs is
very careful with what it does to make sure it doesn't crash the kernel.
> The problem was the not via kmalloc allocated buffer
> ,as you mentioned. Thanks a lot. But dynamically allocated
> memory is very dangerous! reminds me of old dos times :-)
> booting 100times/day
>
>
> ----------------- code starts here -------------------
> /*
> Some USB test
> Hope i wont crash my system
> */
>
> #include "linux/init.h"
> #include "linux/kernel.h"
> #include "linux/module.h"
> #include "linux/slab.h"
>
>
> /* file operarions */
> #include "linux/fs.h"
>
> /* put_user is here */
> #include "asm/uaccess.h"
>
> #include "linux/usb.h"
>
> #include "linux/types.h"
>
> #define USB_VENDOR_TRAVELER 0xd96
> #define USB_PRODUCT_SX330Z 0x3300
>
>
> #define USB_REQ_RESERVED 0x04
> #define USB_DIR_VENDOR 0x40
>
> #define DRIVER_VERSION "v0.1"
> #define DRIVER_DESC "Traveler SX330z Camera Driver"
>
> #define SX330Z_MINOR_BASE 65
>
>
> MODULE_AUTHOR("Dominik Kuhlen <[EMAIL PROTECTED]>");
> MODULE_DESCRIPTION(DRIVER_DESC);
> MODULE_LICENSE("GPL");
>
> struct traveler_sx330z
> {
> int isOpen;
> int connected;
> struct usb_device *dev;
> };
>
>
>
> static struct traveler_sx330z sx330z_instance = {
> isOpen: 0,
> connected: 0,
> dev: NULL,
> }; // this pointer :-)
>
> #define traveler_req_TOC 0x0003
> #define traveler_req_Pic 0x0004
>
> /* 32 Bytes*/
> struct traveler_request
> {
> int16_t always1; // 0x01
> int16_t requesttype; // 0x0003 : TOC , 0x0004 ThumbNail
> unsigned char data[4];
> int32_t timestamp; // counter? only 24 bit
> int32_t offset;
> int32_t size;
> char filename[12];
> } __attribute__((packed));
>
> /* 16 Bytes*/
> struct traveler_ack
> {
> int32_t always3; // 3
> int32_t timestamp; // not sure
> int32_t size; // for TOC
> int32_t dontknow; // always 0
> } __attribute__((packed));
>
>
> /*******************************************************/
> /* Fileoperation OPEN */
> /*******************************************************/
> static unsigned char *TOC; //
> static int TCnt=0;
> static unsigned char *ThumbNail;// 1 Tn: 0x5000 Bytes
I wouldn't use global variables here. It limits you to one device being
used at a time then.
> static int sx330z_open(struct inode *inode,struct file *file)
> {
> struct traveler_sx330z *sx330z=&sx330z_instance;//file->private_data;
> int res,length;
> unsigned char buf[512];
>
> // char *ThumbNail;
>
> struct traveler_request trq;
> struct traveler_ack tack;
> unsigned int pipe;
> int numpics,pics; // pictures
> static int TOCSize=0;
> int TOCPage,tcnt; // TOC SIZE
> int subminor;
> subminor=MINOR(inode->i_rdev)-SX330Z_MINOR_BASE;
> if (sx330z->isOpen) return(-EBUSY); //
> if ((!sx330z->connected)||(subminor!=0)) return(-ENODEV); // Camera not
> connected / powered
> TCnt=0;
> sx330z->isOpen=1;
> MOD_INC_USE_COUNT;
>
>
>
> printk("sx330z : open subminor=%d (vendor=%04x).\n",
> subminor,
> sx330z->dev->descriptor.idVendor);
>
>
> pipe=usb_rcvctrlpipe(sx330z->dev,0);
> res=usb_control_msg(sx330z->dev,
> pipe,
> USB_REQ_RESERVED, // traveler hack ???
> USB_DIR_IN | USB_DIR_VENDOR,
> 0x0001,
> 0x0000,
> &tack,16,HZ);
> printk("sx330z : 0x0001 (res=%d) : %08x %08x %08x %08x\n",
> res,tack.always3,tack.timestamp,tack.size,tack.dontknow);
>
> pipe=usb_rcvctrlpipe(sx330z->dev,0);
> res=usb_control_msg(sx330z->dev,
> pipe,
> USB_REQ_RESERVED, // traveler hack ????
> USB_DIR_IN | USB_DIR_VENDOR,
> 0x0002, // get TOC size ????
> 0xd000,
> &tack,16,HZ);
> printk("sx330z : Get TOCsize (res=%d) : %08x %08x %08x %08x\n",
> res,tack.always3,tack.timestamp,tack.size,tack.dontknow);
Don't use memory allocated on the stack either. kmalloc the memory.
It's just a coincidence that it works on i386, but it won't work on
other architectures.
> TOCSize=tack.size;
> if (TOCSize<0) TOCSize=0x48;
> TOC= kmalloc(TOCSize,GFP_KERNEL); // GFP_KERNEL ?
GFP_KERNEL is correct here.
> if (!TOC) // very dangerous!
> {
> sx330z->isOpen=0;
> MOD_DEC_USE_COUNT; // important
> return(-1);
> }
> printk("sx330z : TOCSize %d Bytes\n",TOCSize);
> /* set ????*/
>
>
> /********************************************************/
> /* TOC x. PAGEs */
> /********************************************************/
> //memset(TOC,0,0x200);
> numpics=0;
> tcnt=0; // toc counter
> for (TOCPage=0;(TOCSize>0)&&(TOCPage<3);TOCPage++)
> {
> trq.always1=1;
> trq.requesttype=0x03; // TOC
> trq.offset=0x0200*TOCPage;
> trq.timestamp=0x0046a7ee +0x20*TOCPage;
> trq.size=0x200;
> memset(trq.data,0,4);
> memset(trq.filename,0,12);
>
> res=usb_control_msg(sx330z->dev,
> usb_sndctrlpipe(sx330z->dev,0),
> USB_REQ_RESERVED, // traveler hack ?????
> USB_DIR_OUT | USB_DIR_VENDOR,
> 0x0003, // read TOC
> 0xd000,
> &trq,32,HZ);
> printk("sx330z : ReqTOCPage %d (res=%d)\n",TOCPage,res);
>
>
> res=usb_bulk_msg(sx330z->dev,usb_rcvbulkpipe(sx330z->dev,2),buf,512,&length,HZ);
> numpics+=pics=buf[0xa]; // how many pics in this fragment
> memcpy(&TOC[tcnt],&buf[0x0c],pics*0x14); // skip 12 bytes
> tcnt+=pics*0x14;
> printk(KERN_INFO "sx330z : read TOC Page #%d %dpics (res=%d,length=%d bytes)
> : %02x %02x %02x %02x ... %02x\n",
> TOCPage,pics,res,length,buf[0],buf[1],buf[2],buf[3],buf[0x10]);
>
> // Get 16Byte Ack
>
> res=usb_bulk_msg(sx330z->dev,usb_rcvbulkpipe(sx330z->dev,2),&tack,16,&length,HZ);
> printk(KERN_INFO "sx330z : read ack (res=%d,length=%d) bytes :%08x %08x %08x
> %08x\n",res,length,
> tack.always3,tack.timestamp,tack.size,tack.dontknow);
>
>
> TOCSize-=(0x0c+pics*0x14);
>
> } // while TOCSize > 0
> if (TOCSize!=0) {printk(KERN_INFO "sx330z : TOCSize corrupted!!!!
> %d\n",TOCSize);}
> printk(KERN_INFO "sx330z : %d Pictures\n",numpics);
You're doing this completely wrong. The buffer you pass to
usb_control_msg and usb_bulk_msg needs to be allocated via kmalloc. If
you're using memcpy after you do the transfer, then the destination can
be anything.
> /************************************************/
> /* 1. ThumbNail */
> /************************************************/
> tcnt=0;
> ThumbNail=kmalloc(5*0x1000,GFP_KERNEL);
> if (!ThumbNail) {return(0);} // skip it ??
Are you sure you shouldn't be calling MODULE_DEC_USE_COUNT here?
> for (tcnt=0;tcnt<5;tcnt++)
> {
> trq.always1=1;
> trq.requesttype=0x04; // Thumbnail
> trq.offset=0;
> trq.size=0x1000;
> trq.timestamp=0x0046a7ee +tcnt*0x41 +0x19f;
> memset(trq.data,0,4);
> sprintf(trq.filename,"SIMG0001JPG");
>
> // request for ThumbNail
> res=usb_control_msg(sx330z->dev,
> usb_sndctrlpipe(sx330z->dev,0),
> USB_REQ_RESERVED, // traveler hack ???
> USB_DIR_OUT | USB_DIR_VENDOR,
> 0x0004,
> 0xd000,
> &trq,32,HZ);
> printk("sx330z : req TN (%d) : \n",res);
>
>
>
>res=usb_bulk_msg(sx330z->dev,usb_rcvbulkpipe(sx330z->dev,2),ThumbNail,4096,&length,HZ);
> printk(KERN_INFO "sx330z : read TN (res=%d,length=%d bytes) : %02x %02x ...
> %02x\n",
> res,length,ThumbNail[0],ThumbNail[1],ThumbNail[0x10]);
Good, you're using ThumbNail, which is allocated via kmalloc, but it
looks like you're just overwriting the data a couple of times.
> res=usb_bulk_msg(sx330z->dev,usb_rcvbulkpipe(sx330z->dev,2),&tack,16,&length,HZ);
> printk(KERN_INFO "sx330z : read ack? (res=%d,length=%d bytes): %08x %08x
> %08x %08x\n",res,length,
> tack.always3,tack.timestamp,tack.size,tack.dontknow);
>
> } // 5 Blocks TN data
>
> return(0); // succeess
> } /* fop open */
>
> /*******************************************************/
> /* Fileoperation RELEASE */
> /*******************************************************/
> static int sx330z_release(struct inode *inode,struct file *file)
> {
> struct traveler_sx330z *sx330z=&sx330z_instance;//file->private_data;
> printk("sx330z : fops.release.\n");
> sx330z->isOpen=0;
> kfree (ThumbNail);
> kfree (TOC);
> MOD_DEC_USE_COUNT;
> return(0);
> }
>
> /*******************************************************/
> /* Fileoperation READ */
> /*******************************************************/
> static ssize_t sx330z_read(struct file *file,char *buffer,size_t count,loff_t
> *ppos)
> {
> ssize_t bytes=0;
>
> printk("sx330z : fops.read %d bytes. \n",count);
>
> while ((count>0))
> {
> put_user(TOC[TCnt],buffer++); // copy message
> count--;
> bytes++;
> TCnt++;
> if (TCnt==1024) TCnt=0;
> }
> return(bytes);
> }
This isn't particularly safe since it won't catch segmentation faults to
the user.
> /*******************************************************/
> /* probe function */
> /*******************************************************/
> void *usb_sx330z_probe(struct usb_device *dev,unsigned intf,const struct
> usb_device_id *id)
> {
> struct traveler_sx330z *sx330z=&sx330z_instance;
> printk(KERN_INFO "SX330z : probe. interface : %d , Speed %d, addr: %d\n",
> intf,dev->speed,dev->devnum);
> if ((dev->descriptor.idVendor!=USB_VENDOR_TRAVELER)||
> (dev->descriptor.idProduct!=USB_PRODUCT_SX330Z)||
> (sx330z->connected)) return(NULL); // only one instance allowed
>
> printk(KERN_INFO "SX330z : Vendor : %04x\n",dev->descriptor.idVendor);
>
> sx330z->dev=dev; // save device context
> sx330z->connected=1;
>
> return(sx330z); // return handler
> }
>
>
> /*******************************************************/
> /* disconnect function */
> /*******************************************************/
> void usb_sx330z_disconnect(struct usb_device *dev,void *ptr)
> {
> struct traveler_sx330z *sx330z=ptr;
> if (sx330z->isOpen) printk(KERN_INFO "SX330z : Warning file open !!\n");
> sx330z->connected=0;
> printk(KERN_INFO "SX330z : disconnect.\n");
> }
>
>
> /*******************************************************/
> /* fileoperations */
> /*******************************************************/
> struct file_operations file_ops = {
> owner: THIS_MODULE,
> open: sx330z_open,
> read: sx330z_read,
> release: sx330z_release,
> };
>
>
> /****************************/
> /* ID Table */
> /****************************/
> static struct usb_device_id usb_sx330z_id_table [] = {
> {USB_DEVICE(USB_VENDOR_TRAVELER,USB_PRODUCT_SX330Z)},
> {} // terminating entry
>
> };
>
>
> MODULE_DEVICE_TABLE(usb,usb_sx330z_id_table);
>
> /***********************/
> /* */
> /**********************/
> static struct usb_driver usb_sx330z_driver = {
> name: "Traveler SX330z",
> probe: usb_sx330z_probe,
> disconnect: usb_sx330z_disconnect,
> id_table: usb_sx330z_id_table,
> fops: &file_ops,
> minor: SX330Z_MINOR_BASE,
> };
>
>
>
>
> /*******************************************************/
> /* Module init */
> /*******************************************************/
> static int __init usb_eumels_init(void)
> {
> int stat;
> stat=usb_register(&usb_ser);
> printk(KERN_INFO "Traveler SX330z USB test loaded (ret=%d).\n",stat);
> if (stat<0) return(-1);
> info(DRIVER_DESC " " DRIVER_VERSION);
> return(0);
> }
>
> /*******************************************************/
> /* Module exit */
> /*******************************************************/
> static void __exit usb_eumels_exit(void)
> {
> usb_deregister(&usb_sx330z_driver);
> printk(KERN_INFO "Traveler SX330z USB test killed.\n" );
> }
>
>
>
> module_init(usb_eumels_init);
> module_exit(usb_eumels_exit);
To be honest it looks more like you have some brushing up on kernel
programming in general than anything. You do lots of unsafe things and
don't properly handle all error conditions correctly.
JE
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
