Skip to content

Commit

Permalink
staging/ft1000-usb: fix problems found by sparse
Browse files Browse the repository at this point in the history 
In the original code, address space annotations are missing,
which hides a possible unchecked user pointer access.

Two functions use a lot of stack space.

Extern declarations are all in the wrong place, which leads
to type differences between caller and callee in some cases.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Arnd Bergmann authored and Greg Kroah-Hartman committed Oct 5, 2010
1 parent 7cfd8a3 commit 2a953cf
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 120 deletions.
36 changes: 15 additions & 21 deletions drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,16 @@
#include "ft1000_usb.h"
//#include "ft1000_ioctl.h"

void ft1000_DestroyDevice(struct net_device *dev);
u16 ft1000_read_dpram16(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer, u8 highlow);
u16 ft1000_read_register(struct ft1000_device *ft1000dev, short* Data, u16 nRegIndx);
static int ft1000_flarion_cnt = 0;

extern inline u16 ft1000_asic_read (struct net_device *dev, u16 offset);
extern inline void ft1000_asic_write (struct net_device *dev, u16 offset, u16 value);
extern void CardSendCommand(struct ft1000_device *ft1000dev, unsigned short *ptempbuffer, int size);
//need to looking usage of ft1000Handle

static int ft1000_ChOpen (struct inode *Inode, struct file *File);
static unsigned int ft1000_ChPoll(struct file *file, poll_table *wait);
static long ft1000_ChIoctl(struct file *File, unsigned int Command,
unsigned long Argument);
static int ft1000_ChRelease (struct inode *Inode, struct file *File);

static int ft1000_flarion_cnt = 0;

//need to looking usage of ft1000Handle



// Global pointer to device object
static struct ft1000_device *pdevobj[MAX_NUM_CARDS + 2];
//static devfs_handle_t ft1000Handle[MAX_NUM_CARDS];
Expand Down Expand Up @@ -326,7 +316,7 @@ int ft1000_CreateDevice(struct ft1000_device *dev)
info->app_info[i].nRxMsg = 0;
info->app_info[i].nTxMsgReject = 0;
info->app_info[i].nRxMsgMiss = 0;
info->app_info[i].fileobject = 0;
info->app_info[i].fileobject = NULL;
info->app_info[i].app_id = i+1;
info->app_info[i].DspBCMsgFlag = 0;
info->app_info[i].NumOfMsg = 0;
Expand Down Expand Up @@ -539,6 +529,7 @@ static unsigned int ft1000_ChPoll(struct file *file, poll_table *wait)
static long ft1000_ChIoctl (struct file *File, unsigned int Command,
unsigned long Argument)
{
void __user *argp = (void __user *)Argument;
struct net_device *dev;
PFT1000_INFO info;
struct ft1000_device *ft1000dev;
Expand Down Expand Up @@ -579,7 +570,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
switch (cmd) {
case IOCTL_REGISTER_CMD:
DEBUG("FT1000:ft1000_ChIoctl: IOCTL_FT1000_REGISTER called\n");
result = get_user(tempword, (unsigned short *)Argument);
result = get_user(tempword, (__u16 __user*)argp);
if (result) {
DEBUG("result = %d failed to get_user\n", result);
break;
Expand All @@ -601,7 +592,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,

get_ver_data.drv_ver = FT1000_DRV_VER;

if (copy_to_user((PIOCTL_GET_VER)Argument, &get_ver_data, sizeof(get_ver_data)) ) {
if (copy_to_user(argp, &get_ver_data, sizeof(get_ver_data)) ) {
DEBUG("FT1000:ft1000_ChIoctl: copy fault occurred\n");
result = -EFAULT;
break;
Expand Down Expand Up @@ -651,7 +642,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
do_gettimeofday ( &tv );
get_stat_data.ConTm = (u32)(tv.tv_sec - info->ConTm);
DEBUG("Connection Time = %d\n", (int)get_stat_data.ConTm);
if (copy_to_user((PIOCTL_GET_DSP_STAT)Argument, &get_stat_data, sizeof(get_stat_data)) ) {
if (copy_to_user(argp, &get_stat_data, sizeof(get_stat_data)) ) {
DEBUG("FT1000:ft1000_ChIoctl: copy fault occurred\n");
result = -EFAULT;
break;
Expand Down Expand Up @@ -692,7 +683,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
//DEBUG("FT1000:ft1000_ChIoctl: try to SET_DPRAM \n");

// Get the length field to see how many bytes to copy
result = get_user(msgsz, (unsigned short *)Argument);
result = get_user(msgsz, (__u16 __user *)argp);
msgsz = ntohs (msgsz);
//DEBUG("FT1000:ft1000_ChIoctl: length of message = %d\n", msgsz);

Expand All @@ -708,7 +699,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
break;

//if ( copy_from_user(&(dpram_command.dpram_blk), (PIOCTL_DPRAM_BLK)Argument, msgsz+2) ) {
if ( copy_from_user(&dpram_data, (PIOCTL_DPRAM_BLK)Argument, msgsz+2) ) {
if ( copy_from_user(&dpram_data, argp, msgsz+2) ) {
DEBUG("FT1000:ft1000_ChIoctl: copy fault occurred\n");
result = -EFAULT;
}
Expand Down Expand Up @@ -852,7 +843,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
}

result = 0;
pioctl_dpram = (PIOCTL_DPRAM_BLK)Argument;
pioctl_dpram = argp;
if (list_empty(&info->app_info[i].app_sqlist) == 0) {
//DEBUG("FT1000:ft1000_ChIoctl:Message detected in slow queue\n");
spin_lock_irqsave(&free_buff_lock, flags);
Expand All @@ -862,7 +853,10 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command,
//DEBUG("FT1000:ft1000_ChIoctl:NumOfMsg for app %d = %d\n", i, info->app_info[i].NumOfMsg);
spin_unlock_irqrestore(&free_buff_lock, flags);
msglen = ntohs(*(u16 *)pdpram_blk->pbuffer) + PSEUDOSZ;
pioctl_dpram->total_len = htons(msglen); /* XXX exploit here */
result = get_user(msglen, &pioctl_dpram->total_len);
if (result)
break;
msglen = htons(msglen);
//DEBUG("FT1000:ft1000_ChIoctl:msg length = %x\n", msglen);
if(copy_to_user (&pioctl_dpram->pseudohdr, pdpram_blk->pbuffer, msglen))
{
Expand Down Expand Up @@ -935,7 +929,7 @@ static int ft1000_ChRelease (struct inode *Inode, struct file *File)
// initialize application information
info->appcnt--;
DEBUG("ft1000_chdev:%s:appcnt = %d\n", __FUNCTION__, info->appcnt);
info->app_info[i].fileobject = 0;
info->app_info[i].fileobject = NULL;

return 0;
}
Expand Down
41 changes: 17 additions & 24 deletions drivers/staging/ft1000/ft1000-usb/ft1000_download.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,6 @@ typedef struct _DSP_IMAGE_INFO_V6 {
} DSP_IMAGE_INFO_V6, *PDSP_IMAGE_INFO_V6;


u16 ft1000_read_register(struct ft1000_device *ft1000dev, short* Data, u16 nRegIndx);
u16 ft1000_write_register(struct ft1000_device *ft1000dev, USHORT value, u16 nRegIndx);
u16 ft1000_read_dpram32(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer, USHORT cnt);
u16 ft1000_write_dpram32(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer, USHORT cnt);
u16 ft1000_read_dpram16(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer, u8 highlow);
u16 ft1000_write_dpram16(struct ft1000_device *ft1000dev, USHORT indx, USHORT value, u8 highlow);
u16 fix_ft1000_read_dpram32(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer);
u16 fix_ft1000_write_dpram32(struct ft1000_device *ft1000dev, USHORT indx, PUCHAR buffer);

//---------------------------------------------------------------------------
// Function: getfw
//
Expand All @@ -154,7 +145,7 @@ u16 fix_ft1000_write_dpram32(struct ft1000_device *ft1000dev, USHORT indx, PUCHA
// Notes:
//
//---------------------------------------------------------------------------
char *getfw (char *fn, int *pimgsz)
char *getfw (char *fn, size_t *pimgsz)
{
struct file *fd;
mm_segment_t fs = get_fs();
Expand Down Expand Up @@ -190,7 +181,7 @@ char *getfw (char *fn, int *pimgsz)
return NULL;
}
pos = 0;
if (vfs_read(fd, pfwimg, fwimgsz, &pos) != fwimgsz) {
if (vfs_read(fd, (void __user __force*)pfwimg, fwimgsz, &pos) != fwimgsz) {
vfree(pfwimg);
DEBUG("FT1000:%s:failed to read firmware image\n",__FUNCTION__);
filp_close(fd, current->files);
Expand All @@ -216,7 +207,7 @@ char *getfw (char *fn, int *pimgsz)
// Notes:
//
//---------------------------------------------------------------------------
ULONG check_usb_db (struct ft1000_device *ft1000dev)
static ULONG check_usb_db (struct ft1000_device *ft1000dev)
{
int loopcnt;
USHORT temp;
Expand Down Expand Up @@ -295,7 +286,7 @@ ULONG check_usb_db (struct ft1000_device *ft1000dev)
// Notes:
//
//---------------------------------------------------------------------------
USHORT get_handshake(struct ft1000_device *ft1000dev, USHORT expected_value)
static USHORT get_handshake(struct ft1000_device *ft1000dev, USHORT expected_value)
{
USHORT handshake;
int loopcnt;
Expand Down Expand Up @@ -406,7 +397,7 @@ USHORT get_handshake(struct ft1000_device *ft1000dev, USHORT expected_value)
// Notes:
//
//---------------------------------------------------------------------------
void put_handshake(struct ft1000_device *ft1000dev,USHORT handshake_value)
static void put_handshake(struct ft1000_device *ft1000dev,USHORT handshake_value)
{
ULONG tempx;
USHORT tempword;
Expand Down Expand Up @@ -442,7 +433,7 @@ void put_handshake(struct ft1000_device *ft1000dev,USHORT handshake_value)

}

USHORT get_handshake_usb(struct ft1000_device *ft1000dev, USHORT expected_value)
static USHORT get_handshake_usb(struct ft1000_device *ft1000dev, USHORT expected_value)
{
USHORT handshake;
int loopcnt;
Expand Down Expand Up @@ -482,7 +473,7 @@ USHORT get_handshake_usb(struct ft1000_device *ft1000dev, USHORT expected_value)
return HANDSHAKE_TIMEOUT_VALUE;
}

void put_handshake_usb(struct ft1000_device *ft1000dev,USHORT handshake_value)
static void put_handshake_usb(struct ft1000_device *ft1000dev,USHORT handshake_value)
{
int i;

Expand All @@ -501,7 +492,7 @@ void put_handshake_usb(struct ft1000_device *ft1000dev,USHORT handshake_value)
// Notes:
//
//---------------------------------------------------------------------------
USHORT get_request_type(struct ft1000_device *ft1000dev)
static USHORT get_request_type(struct ft1000_device *ft1000dev)
{
USHORT request_type;
ULONG status;
Expand Down Expand Up @@ -533,7 +524,7 @@ USHORT get_request_type(struct ft1000_device *ft1000dev)

}

USHORT get_request_type_usb(struct ft1000_device *ft1000dev)
static USHORT get_request_type_usb(struct ft1000_device *ft1000dev)
{
USHORT request_type;
ULONG status;
Expand Down Expand Up @@ -577,7 +568,7 @@ USHORT get_request_type_usb(struct ft1000_device *ft1000dev)
// Notes:
//
//---------------------------------------------------------------------------
long get_request_value(struct ft1000_device *ft1000dev)
static long get_request_value(struct ft1000_device *ft1000dev)
{
ULONG value;
USHORT tempword;
Expand Down Expand Up @@ -605,7 +596,8 @@ long get_request_value(struct ft1000_device *ft1000dev)

}

long get_request_value_usb(struct ft1000_device *ft1000dev)
#if 0
static long get_request_value_usb(struct ft1000_device *ft1000dev)
{
ULONG value;
USHORT tempword;
Expand Down Expand Up @@ -633,6 +625,7 @@ long get_request_value_usb(struct ft1000_device *ft1000dev)
return value;

}
#endif

//---------------------------------------------------------------------------
// Function: put_request_value
Expand All @@ -647,7 +640,7 @@ long get_request_value_usb(struct ft1000_device *ft1000dev)
// Notes:
//
//---------------------------------------------------------------------------
void put_request_value(struct ft1000_device *ft1000dev, long lvalue)
static void put_request_value(struct ft1000_device *ft1000dev, long lvalue)
{
ULONG tempx;
ULONG status;
Expand Down Expand Up @@ -675,7 +668,7 @@ void put_request_value(struct ft1000_device *ft1000dev, long lvalue)
// Notes:
//
//---------------------------------------------------------------------------
USHORT hdr_checksum(PPSEUDO_HDR pHdr)
static USHORT hdr_checksum(PPSEUDO_HDR pHdr)
{
USHORT *usPtr = (USHORT *)pHdr;
USHORT chksum;
Expand Down Expand Up @@ -705,7 +698,7 @@ USHORT hdr_checksum(PPSEUDO_HDR pHdr)
// Notes:
//
//---------------------------------------------------------------------------
ULONG write_blk (struct ft1000_device *ft1000dev, USHORT **pUsFile, UCHAR **pUcFile, long word_length)
static ULONG write_blk (struct ft1000_device *ft1000dev, USHORT **pUsFile, UCHAR **pUcFile, long word_length)
{
ULONG Status = STATUS_SUCCESS;
USHORT dpram;
Expand Down Expand Up @@ -861,7 +854,7 @@ static void usb_dnld_complete (struct urb *urb)
// Notes:
//
//---------------------------------------------------------------------------
ULONG write_blk_fifo (struct ft1000_device *ft1000dev, USHORT **pUsFile, UCHAR **pUcFile, long word_length)
static ULONG write_blk_fifo (struct ft1000_device *ft1000dev, USHORT **pUsFile, UCHAR **pUcFile, long word_length)
{
ULONG Status = STATUS_SUCCESS;
int byte_length;
Expand Down
Loading

0 comments on commit 2a953cf

Please sign in to comment.