lkml.org 
[lkml]   [2007]   [May]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] Fix/add raw1394 CONFIG_COMPAT code
    Date
    On Sunday 06 May 2007 19:14, Petr Vandrovec wrote:
    > Hello Dan,
    >
    > This patch makes raw1394 in current Linux git tree (2.6.21-1570) usable to 32bit
    > applications running on 64bit kernel (tested on i386 app using x86_64 kernel). I
    > had to make following changes:
    >
    > * read() always failed with -EFAULT. This was happening due to
    > raw1394_compat_read copying data to wrong location - access_ok always
    > failed as 'r' is kernel address, not user. Whole function just tried to
    > copy data from 'r' to 'r', which is not good.
    >
    > * write(fd, buf, 52) from 32bit app was returning 56. Most of callers did not
    > care, but some (arm registration) did, and anyway it looks bad if request for
    > writing 52 bytes returns 56. And returning sizeof anything in 'int' is not
    > good as well. So all functions now return '0' instead of
    > sizeof(struct raw1394_request) on success, and write() itself provides correct
    > return value (it just returns value it was asked to write on success as raw1394
    > does not do any partial writes at all).
    >
    > * Related to this was problem that write() could have returned 0 when kernel
    > state would become corrupted and moved to different state than
    > opened/initialized/connected. Now it returns -EBADFD which seemed appropriate.
    >
    > * And add compat_ioctl. Although all structures are more or less same,
    > raw1394_iso_packets got pointer inside, and raw1394_cycle_timer got unwanted
    > padding in the middle. I did not add any translation for ioctls passing array
    > of integers around as integers seem to have same size (32 bits) on all
    > architectures supported by Linux.
    >
    > With this in place I was able to run my test app and grab some mpegs, so I believe
    > that I got everything necessary done. If you believe I have missed some ioctl, yell
    > at me.
    > Thanks,
    > Petr Vandrovec
    >
    > Signed-off-by: Petr Vandrovec <petr@vandrovec.name>

    Reviewed and been testing out fine here on i386.
    Acked-by: Dan Dennedy <dan@dennedy.org>

    > diff -uprdN linux/drivers/ieee1394/raw1394.c linux/drivers/ieee1394/raw1394.c
    > --- linux/drivers/ieee1394/raw1394.c 2007-05-06 17:45:47.000000000 -0700
    > +++ linux/drivers/ieee1394/raw1394.c 2007-05-06 18:08:05.000000000 -0700
    > @@ -460,7 +460,7 @@ static const char __user *raw1394_compat
    > static int
    > raw1394_compat_read(const char __user *buf, struct raw1394_request *r)
    > {
    > - struct compat_raw1394_req __user *cr = (typeof(cr)) r;
    > + struct compat_raw1394_req __user *cr = (typeof(cr)) buf;
    > if (!access_ok(VERIFY_WRITE, cr, sizeof(struct compat_raw1394_req)) ||
    > P(type) ||
    > P(error) ||
    > @@ -588,7 +588,7 @@ static int state_opened(struct file_info
    >
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int state_initialized(struct file_info *fi, struct pending_request *req)
    > @@ -602,7 +602,7 @@ static int state_initialized(struct file
    > req->req.generation = atomic_read(&internal_generation);
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > switch (req->req.type) {
    > @@ -674,7 +674,7 @@ out_set_card:
    > }
    >
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static void handle_iso_listen(struct file_info *fi, struct pending_request *req)
    > @@ -866,7 +866,7 @@ static int handle_async_request(struct f
    > if (req->req.error) {
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > hpsb_set_packet_complete_task(packet,
    > @@ -884,7 +884,7 @@ static int handle_async_request(struct f
    > hpsb_free_tlabel(packet);
    > queue_complete_req(req);
    > }
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int handle_iso_send(struct file_info *fi, struct pending_request *req,
    > @@ -908,7 +908,7 @@ static int handle_iso_send(struct file_i
    > req->req.error = RAW1394_ERROR_MEMFAULT;
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > req->req.length = 0;
    > @@ -928,7 +928,7 @@ static int handle_iso_send(struct file_i
    > queue_complete_req(req);
    > }
    >
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int handle_async_send(struct file_info *fi, struct pending_request *req)
    > @@ -943,7 +943,7 @@ static int handle_async_send(struct file
    > req->req.error = RAW1394_ERROR_INVALID_ARG;
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > packet = hpsb_alloc_packet(req->req.length - header_length);
    > @@ -956,7 +956,7 @@ static int handle_async_send(struct file
    > req->req.error = RAW1394_ERROR_MEMFAULT;
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > if (copy_from_user
    > @@ -965,7 +965,7 @@ static int handle_async_send(struct file
    > req->req.error = RAW1394_ERROR_MEMFAULT;
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > packet->type = hpsb_async;
    > @@ -993,7 +993,7 @@ static int handle_async_send(struct file
    > queue_complete_req(req);
    > }
    >
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int arm_read(struct hpsb_host *host, int nodeid, quadlet_t * buffer,
    > @@ -1868,7 +1868,7 @@ static int arm_register(struct file_info
    > spin_lock_irqsave(&host_info_lock, flags);
    > list_add_tail(&addr->addr_list, &fi->addr_list);
    > spin_unlock_irqrestore(&host_info_lock, flags);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > retval =
    > hpsb_register_addrspace(&raw1394_highlevel, fi->host, &arm_ops,
    > @@ -1886,7 +1886,7 @@ static int arm_register(struct file_info
    > return (-EALREADY);
    > }
    > free_pending_request(req); /* immediate success or fail */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int arm_unregister(struct file_info *fi, struct pending_request *req)
    > @@ -1954,7 +1954,7 @@ static int arm_unregister(struct file_in
    > vfree(addr->addr_space_buffer);
    > kfree(addr);
    > free_pending_request(req); /* immediate success or fail */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > retval =
    > hpsb_unregister_addrspace(&raw1394_highlevel, fi->host,
    > @@ -1970,7 +1970,7 @@ static int arm_unregister(struct file_in
    > vfree(addr->addr_space_buffer);
    > kfree(addr);
    > free_pending_request(req); /* immediate success or fail */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > /* Copy data from ARM buffer(s) to user buffer. */
    > @@ -2012,7 +2012,7 @@ static int arm_get_buf(struct file_info
    > * queue no response, and therefore nobody
    > * will free it. */
    > free_pending_request(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > } else {
    > DBGMSG("arm_get_buf request exceeded mapping");
    > spin_unlock_irqrestore(&host_info_lock, flags);
    > @@ -2064,7 +2064,7 @@ static int arm_set_buf(struct file_info
    > * queue no response, and therefore nobody
    > * will free it. */
    > free_pending_request(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > } else {
    > DBGMSG("arm_set_buf request exceeded mapping");
    > spin_unlock_irqrestore(&host_info_lock, flags);
    > @@ -2085,7 +2085,7 @@ static int reset_notification(struct fil
    > (req->req.misc == RAW1394_NOTIFY_ON)) {
    > fi->notification = (u8) req->req.misc;
    > free_pending_request(req); /* we have to free the request, because we queue no response, and therefore nobody will free it */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > /* error EINVAL (22) invalid argument */
    > return (-EINVAL);
    > @@ -2118,12 +2118,12 @@ static int write_phypacket(struct file_i
    > req->req.length = 0;
    > queue_complete_req(req);
    > }
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > static int get_config_rom(struct file_info *fi, struct pending_request *req)
    > {
    > - int ret = sizeof(struct raw1394_request);
    > + int ret = 0;
    > quadlet_t *data = kmalloc(req->req.length, GFP_KERNEL);
    > int status;
    >
    > @@ -2153,7 +2153,7 @@ static int get_config_rom(struct file_in
    >
    > static int update_config_rom(struct file_info *fi, struct pending_request *req)
    > {
    > - int ret = sizeof(struct raw1394_request);
    > + int ret = 0;
    > quadlet_t *data = kmalloc(req->req.length, GFP_KERNEL);
    > if (!data)
    > return -ENOMEM;
    > @@ -2220,7 +2220,7 @@ static int modify_config_rom(struct file
    >
    > hpsb_update_config_rom_image(fi->host);
    > free_pending_request(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > }
    >
    > @@ -2285,7 +2285,7 @@ static int modify_config_rom(struct file
    > /* we have to free the request, because we queue no response,
    > * and therefore nobody will free it */
    > free_pending_request(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > } else {
    > for (dentry =
    > fi->csr1212_dirs[dr]->value.directory.dentries_head;
    > @@ -2310,7 +2310,7 @@ static int state_connected(struct file_i
    >
    > case RAW1394_REQ_ECHO:
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    >
    > case RAW1394_REQ_ISO_SEND:
    > print_old_iso_deprecation();
    > @@ -2334,24 +2334,24 @@ static int state_connected(struct file_i
    > case RAW1394_REQ_ISO_LISTEN:
    > print_old_iso_deprecation();
    > handle_iso_listen(fi, req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    >
    > case RAW1394_REQ_FCP_LISTEN:
    > handle_fcp_listen(fi, req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    >
    > case RAW1394_REQ_RESET_BUS:
    > if (req->req.misc == RAW1394_LONG_RESET) {
    > DBGMSG("busreset called (type: LONG)");
    > hpsb_reset_bus(fi->host, LONG_RESET);
    > free_pending_request(req); /* we have to free the request, because we queue no response, and therefore nobody will free it */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > if (req->req.misc == RAW1394_SHORT_RESET) {
    > DBGMSG("busreset called (type: SHORT)");
    > hpsb_reset_bus(fi->host, SHORT_RESET);
    > free_pending_request(req); /* we have to free the request, because we queue no response, and therefore nobody will free it */
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    > /* error EINVAL (22) invalid argument */
    > return (-EINVAL);
    > @@ -2370,7 +2370,7 @@ static int state_connected(struct file_i
    > req->req.generation = get_hpsb_generation(fi->host);
    > req->req.length = 0;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > switch (req->req.type) {
    > @@ -2383,7 +2383,7 @@ static int state_connected(struct file_i
    > if (req->req.length == 0) {
    > req->req.error = RAW1394_ERROR_INVALID_ARG;
    > queue_complete_req(req);
    > - return sizeof(struct raw1394_request);
    > + return 0;
    > }
    >
    > return handle_async_request(fi, req, node);
    > @@ -2394,7 +2394,7 @@ static ssize_t raw1394_write(struct file
    > {
    > struct file_info *fi = (struct file_info *)file->private_data;
    > struct pending_request *req;
    > - ssize_t retval = 0;
    > + ssize_t retval = -EBADFD;
    >
    > #ifdef CONFIG_COMPAT
    > if (count == sizeof(struct compat_raw1394_req) &&
    > @@ -2436,6 +2436,9 @@ static ssize_t raw1394_write(struct file
    >
    > if (retval < 0) {
    > free_pending_request(req);
    > + } else {
    > + BUG_ON(retval);
    > + retval = count;
    > }
    >
    > return retval;
    > @@ -2801,6 +2804,99 @@ static int raw1394_ioctl(struct inode *i
    > return -EINVAL;
    > }
    >
    > +#ifdef CONFIG_COMPAT
    > +struct raw1394_iso_packets32 {
    > + __u32 n_packets;
    > + compat_uptr_t infos;
    > +} __attribute__((packed));
    > +
    > +struct raw1394_cycle_timer32 {
    > + __u32 cycle_timer;
    > + __u64 local_time;
    > +} __attribute__((packed));
    > +
    > +#define RAW1394_IOC_ISO_RECV_PACKETS32 \
    > + _IOW ('#', 0x25, struct raw1394_iso_packets32)
    > +#define RAW1394_IOC_ISO_XMIT_PACKETS32 \
    > + _IOW ('#', 0x27, struct raw1394_iso_packets32)
    > +#define RAW1394_IOC_GET_CYCLE_TIMER32 \
    > + _IOR ('#', 0x30, struct raw1394_cycle_timer32)
    > +
    > +static long raw1394_iso_xmit_recv_packets32(struct file *file, unsigned int cmd,
    > + struct raw1394_iso_packets32 __user *arg)
    > +{
    > + compat_uptr_t infos32;
    > + void *infos;
    > + long err = -EFAULT;
    > + struct raw1394_iso_packets __user *dst = compat_alloc_user_space(sizeof(struct raw1394_iso_packets));
    > +
    > + if (!copy_in_user(&dst->n_packets, &arg->n_packets, sizeof arg->n_packets) &&
    > + !copy_from_user(&infos32, &arg->infos, sizeof infos32)) {
    > + infos = compat_ptr(infos32);
    > + if (!copy_to_user(&dst->infos, &infos, sizeof infos))
    > + err = raw1394_ioctl(NULL, file, cmd, (unsigned long)dst);
    > + }
    > + return err;
    > +}
    > +
    > +static long raw1394_read_cycle_timer32(struct file_info *fi, void __user * uaddr)
    > +{
    > + struct raw1394_cycle_timer32 ct;
    > + int err;
    > +
    > + err = hpsb_read_cycle_timer(fi->host, &ct.cycle_timer, &ct.local_time);
    > + if (!err)
    > + if (copy_to_user(uaddr, &ct, sizeof(ct)))
    > + err = -EFAULT;
    > + return err;
    > +}
    > +
    > +static long raw1394_compat_ioctl(struct file *file,
    > + unsigned int cmd, unsigned long arg)
    > +{
    > + struct file_info *fi = file->private_data;
    > + void __user *argp = (void __user *)arg;
    > + long err;
    > +
    > + lock_kernel();
    > + switch (cmd) {
    > + /* These requests have same format as long as 'int' has same size. */
    > + case RAW1394_IOC_ISO_RECV_INIT:
    > + case RAW1394_IOC_ISO_RECV_START:
    > + case RAW1394_IOC_ISO_RECV_LISTEN_CHANNEL:
    > + case RAW1394_IOC_ISO_RECV_UNLISTEN_CHANNEL:
    > + case RAW1394_IOC_ISO_RECV_SET_CHANNEL_MASK:
    > + case RAW1394_IOC_ISO_RECV_RELEASE_PACKETS:
    > + case RAW1394_IOC_ISO_RECV_FLUSH:
    > + case RAW1394_IOC_ISO_XMIT_RECV_STOP:
    > + case RAW1394_IOC_ISO_XMIT_INIT:
    > + case RAW1394_IOC_ISO_XMIT_START:
    > + case RAW1394_IOC_ISO_XMIT_SYNC:
    > + case RAW1394_IOC_ISO_GET_STATUS:
    > + case RAW1394_IOC_ISO_SHUTDOWN:
    > + case RAW1394_IOC_ISO_QUEUE_ACTIVITY:
    > + err = raw1394_ioctl(NULL, file, cmd, arg);
    > + break;
    > + /* These request have different format. */
    > + case RAW1394_IOC_ISO_RECV_PACKETS32:
    > + err = raw1394_iso_xmit_recv_packets32(file, RAW1394_IOC_ISO_RECV_PACKETS, argp);
    > + break;
    > + case RAW1394_IOC_ISO_XMIT_PACKETS32:
    > + err = raw1394_iso_xmit_recv_packets32(file, RAW1394_IOC_ISO_XMIT_PACKETS, argp);
    > + break;
    > + case RAW1394_IOC_GET_CYCLE_TIMER32:
    > + err = raw1394_read_cycle_timer32(fi, argp);
    > + break;
    > + default:
    > + err = -EINVAL;
    > + break;
    > + }
    > + unlock_kernel();
    > +
    > + return err;
    > +}
    > +#endif
    > +
    > static unsigned int raw1394_poll(struct file *file, poll_table * pt)
    > {
    > struct file_info *fi = file->private_data;
    > @@ -3040,7 +3136,9 @@ static const struct file_operations raw1
    > .write = raw1394_write,
    > .mmap = raw1394_mmap,
    > .ioctl = raw1394_ioctl,
    > - // .compat_ioctl = ... someone needs to do this
    > +#ifdef CONFIG_COMPAT
    > + .compat_ioctl = raw1394_compat_ioctl,
    > +#endif
    > .poll = raw1394_poll,
    > .open = raw1394_open,
    > .release = raw1394_release,
    >
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-05-15 05:55    [W:0.070 / U:91.148 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site