Re: simplify procfs code for seq_file instances V3

2018-05-16 Thread Al Viro
On Wed, May 16, 2018 at 11:43:04AM +0200, Christoph Hellwig wrote:
> We currently have hundreds of proc files that implement plain, read-only
> seq_file based interfaces.  This series consolidates them using new
> procfs helpers that take the seq_operations or simple show callback
> directly.
> 
> A git tree is available at:
> 
> git://git.infradead.org/users/hch/misc.git proc_create.3

Pulled, but the last bit is a bleeding atrocity in need of followup
cleanup.


Re: simplify procfs code for seq_file instances V2

2018-05-06 Thread Al Viro
On Sun, May 06, 2018 at 08:19:49PM +0300, Alexey Dobriyan wrote:
> +++ b/fs/proc/internal.h
> @@ -48,8 +48,8 @@ struct proc_dir_entry {
>   const struct seq_operations *seq_ops;
>   int (*single_show)(struct seq_file *, void *);
>   };
> - unsigned int state_size;
>   void *data;
> + unsigned int state_size;
>   unsigned int low_ino;
>   nlink_t nlink;
>   kuid_t uid;

Makes sense

> @@ -62,9 +62,9 @@ struct proc_dir_entry {
>   umode_t mode;
>   u8 namelen;
>  #ifdef CONFIG_64BIT
> -#define SIZEOF_PDE_INLINE_NAME   (192-139)
> +#define SIZEOF_PDE_INLINE_NAME   (192-155)
>  #else
> -#define SIZEOF_PDE_INLINE_NAME   (128-87)
> +#define SIZEOF_PDE_INLINE_NAME   (128-95)

>  #endif
>   char inline_name[SIZEOF_PDE_INLINE_NAME];
>  } __randomize_layout;

*UGH*

Both to the original state and that kind of "adjustments".
Incidentally, with __bugger_layout in there these expressions
are simply wrong.

If nothing else, I would suggest turning the last one into
char inline_name[];
in hope that layout won't get... randomized that much and
used

#ifdef CONFIG_64BIT
#define PDE_SIZE 192
#else
#define PDE_SIZE 128
#endif

union __proc_dir_entry {
char pad[PDE_SIZE];
struct proc_dir_entry real;
};

#define SIZEOF_PDE_INLINE_NAME (PDE_SIZE - offsetof(struct proc_dir_entry, 
inline_name))

for constants, adjusted sizeof and sizeof_field when creating
proc_dir_entry_cache and turned proc_root into

union __proc_dir_entry __proc_root = { .real = {
.low_ino= PROC_ROOT_INO,
.namelen= 5,
.mode   = S_IFDIR | S_IRUGO | S_IXUGO,
.nlink  = 2,
.refcnt = REFCOUNT_INIT(1),
.proc_iops  = _root_inode_operations,
.proc_fops  = _root_operations,
.parent = &__proc_root.real,
.subdir = RB_ROOT,
.name   = __proc_root.real.inline_name,
.inline_name= "/proc",
}};

#define proc_root __proc_root.real
(or actually used __proc_root.real in all of a 6 places where it remains).

> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index baf1994289ce..7d94fa005b0d 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -40,7 +40,7 @@ static struct net *get_proc_net(const struct inode *inode)
>  
>  static int seq_open_net(struct inode *inode, struct file *file)
>  {
> - size_t state_size = PDE(inode)->state_size;
> + unsigned int state_size = PDE(inode)->state_size;
>   struct seq_net_private *p;
>   struct net *net;


You and your "size_t is evil" crusade...


[RFC] apparently broken error recovery in vhost_scsi_iov_to_sgl()

2017-09-24 Thread Al Viro
Suppose vhost_scsi_iov_to_sgl() got a two-iovec array, mapped
e.g. 20 pages from the first one just fine and failed on the
second.

static int
vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, bool write,
  struct iov_iter *iter,
  struct scatterlist *sg, int sg_count)
{
size_t off = iter->iov_offset;
int i, ret;

for (i = 0; i < iter->nr_segs; i++) {
void __user *base = iter->iov[i].iov_base + off;
size_t len = iter->iov[i].iov_len - off;

ret = vhost_scsi_map_to_sgl(cmd, base, len, sg, write);
if (ret < 0) {
for (i = 0; i < sg_count; i++) {
struct page *page = sg_page([i]);
if (page)
put_page(page);
}
return ret;
}
sg += ret;
off = 0;
}
return 0;
}

What are we trying to drop in the if (ret < 0) in there?  In the case
above we step into it on the second pass through the loop.  The first
20 entries of sg had been filled... and sg had been increased by 20,
so whatever we find and feed to put_page(), it won't be those 20 pages.
Moreover, the caller will reset cmd->tvc_{prot_,}sgl_count to zero,
so vhost_scsi_release_cmd() won't find them either.

Am I missing something subtle here, or should that thing be doing
something like

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 046f6d280af5..e47c5bc3ddca 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -688,6 +688,7 @@ vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, bool 
write,
  struct scatterlist *sg, int sg_count)
 {
size_t off = iter->iov_offset;
+   struct scatterlist *p = sg;
int i, ret;
 
for (i = 0; i < iter->nr_segs; i++) {
@@ -696,8 +697,8 @@ vhost_scsi_iov_to_sgl(struct vhost_scsi_cmd *cmd, bool 
write,
 
ret = vhost_scsi_map_to_sgl(cmd, base, len, sg, write);
if (ret < 0) {
-   for (i = 0; i < sg_count; i++) {
-   struct page *page = sg_page([i]);
+   while (p < sg) {
+   struct page *page = sg_page(p++);
if (page)
put_page(page);
}


Re: [PATCH] mpt3sas: downgrade full copy_from_user to access_ok check

2017-09-20 Thread Al Viro
On Tue, Sep 19, 2017 at 11:11:11PM -0400, Meng Xu wrote:
> Since right after the user copy, we are going to
> memset(, 0, sizeof(karg)), I guess an access_ok check is enough?

access_ok() is *NOT* "will copy_from_user() succeed?"  Not even close.
On a bunch of architectures (sparc64, for one) access_ok() is always
true.

All it does is checking that address is not a kernel one - e.g. on
i386 anything in range 0..3Gb qualifies.  Whether anything's mapped
at that address or not.

Why bother with that copy_from_user() at all?  The same ioctl()
proceeds to copy_to_user() on exact same range; all you get from
it is "if the area passed by caller is writable, but not readable,
fail with -EFAULT".  Who cares?

Just drop that copy_from_user() completely.  Anything access_ok()
might've caught will be caught by copy_to_user() anyway.


Re: [PATCH 1/4] scsi: pmcraid: use __iomem pointers for ioctl argument

2017-04-20 Thread Al Viro
On Thu, Apr 20, 2017 at 07:54:45PM +0200, Arnd Bergmann wrote:
> kernelci.org reports a new compile warning for old code in the pmcraid
> driver:
> 
> arch/mips/include/asm/uaccess.h:138:21: warning: passing argument 1 of 
> '__access_ok' makes pointer from integer without a cast [-Wint-conversion]
> 
> The warning got introduced by a cleanup to the access_ok() helper
> that requires the argument to be a pointer, where the old version
> silently accepts 'unsigned long' arguments as it still does on most
> other architectures.
> 
> The new behavior in MIPS however seems absolutely sensible, and so far I
> could only find one other file with the same issue, so the best solution
> seems to be to clean up the pmcraid driver.
> 
> This makes the driver consistently use 'void __iomem *' pointers for
> passing around the address of the user space ioctl arguments, which gets
> rid of the kernelci warning as well as several sparse warnings.

Is there any point in keeping that access_ok() in the first place, rather
than just switching to copy_from_user()/copy_to_user() in there?  AFAICS,
it's only for the sake of the loop in pmcraid_copy_sglist():
for (i = 0; i < (len / bsize_elem); i++, buffer += bsize_elem) {
struct page *page = sg_page([i]);

kaddr = kmap(page);
if (direction == DMA_TO_DEVICE)
rc = __copy_from_user(kaddr,
  (void *)buffer,
  bsize_elem);
else   
rc = __copy_to_user((void *)buffer, kaddr, bsize_elem);

kunmap(page);

if (rc) {
pmcraid_err("failed to copy user data into sg list\n");
return -EFAULT;
}

scatterlist[i].length = bsize_elem;
}   
and seeing that each of those calls copies is at least a full page...  If
there is an architecture where a single access_ok() costs a noticable fraction
of the time it takes to copy a full page, we have a much worse problem than
overhead in obscure ioctl...


Re: scsi: BUG in scsi_init_io

2017-02-18 Thread Al Viro
On Tue, Jan 31, 2017 at 07:41:51AM -0800, James Bottomley wrote:

> > Please-please-please, let's not use WARN for something that is not a
> > kernel bug and is user-triggerable.
> 
> It is a kernel bug and it should not be user triggerable, so it should
> have a warn_on or bug_on.  It means something called a data setup
> function with no data.  There's actually a root cause that patches like
> this won't fix, can we find it?

The root cause is unfixable without access to TARDIS and dose of
antipsychotics sufficient to prevent /dev/sg API creation.

What happens is that write to /dev/sg is given a request with non-zero
->iovec_count combined with zero ->dxfer_len.  Or with ->dxferp pointing
to an array full of empty iovecs.

AFAICS, the minimal fix would be something like this:

YAMissingSanityCheck in /dev/sg

write permission to /dev/sg shouldn't be equivalent to the ability to trigger
BUG_ON() while holding spinlocks...

Cc: sta...@vger.kernel.org
Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
---

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index dbe5b4b95df0..121de0aaa6ad 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1753,6 +1753,10 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
return res;
 
iov_iter_truncate(, hp->dxfer_len);
+   if (!iov_iter_count()) {
+   kfree(iov);
+   return -EINVAL;
+   }
 
res = blk_rq_map_user_iov(q, rq, md, , GFP_ATOMIC);
kfree(iov);


Re: scsi: use-after-free in bio_copy_from_iter

2016-12-05 Thread Al Viro
On Mon, Dec 05, 2016 at 04:17:53PM +0100, Johannes Thumshirn wrote:
> 633 hp = >header;
> [...]
> 646 hp->dxferp = (char __user *)buf + cmd_size;

> So the memory for hp->dxferp comes from:
> 633 hp = >header;



> >From my debug instrumentation I see that the dxferp ends up in the
> iovec_iter's kvec->iov_base and the faulting address is always dxferp + n *
> 4k with n in [1, 16] (and we're copying 16 4k pages from the iovec into the
> bio).

_Address_ of hp->dxferp comes from that assignment; the value is 'buf'
argument of sg_write() + small offset.  In this case, it should point
inside a pipe buffer, which is, indeed, at a kernel address.  Who'd
allocated srp is irrelevant.

And if you end up dereferencing more than one page worth there, you do have
a problem - pipe buffers are not going to be that large.  Could you slap
WARN_ON((size_t)input_size > count);
right after the calculation of input_size in sg_write() and see if it triggers
on your reproducer?
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH-v3 5/9] vhost/scsi: Add ANY_LAYOUT vhost_virtqueue callback

2015-02-03 Thread Al Viro
On Tue, Feb 03, 2015 at 06:29:59AM +, Nicholas A. Bellinger wrote:
 +  * Copy over the virtio-scsi request header, which when
 +  * ANY_LAYOUT is enabled may span multiple iovecs, or a
 +  * single iovec may contain both the header + outgoing
 +  * WRITE payloads.
 +  *
 +  * copy_from_iter() is modifying the iovecs as copies over
 +  * req_size bytes into req, so the returned out_iter.iov[0]
 +  * will contain the correct start + offset of the outgoing
 +  * WRITE payload, if DMA_TO_DEVICE is set.

It does no such thing.  What it does, though, is changing out_iter so
that subsequent copy_from_iter() will return the data you want.  Note
that out_iter.iov[0] will contain the correct _segment_ of that vector,
with the data you want at out_iter.iov_offset bytes from the beginning
of that segment.  .iov may come to point to subsequent segments and .iov_offset
keeps changing, but segments themselves are never changed.

 +  */
 + iov_iter_init(out_iter, READ, vq-iov[0], out,
  WRITE, please - as in this is
the source of some write, we'll be copying _from_ it.  READ would be
destination of some read, we'll be copying into it.

 +  (data_direction == DMA_TO_DEVICE) ?
 +   req_size + exp_data_len : req_size);
 +
 + ret = copy_from_iter(req, req_size, out_iter);

...

 + /*
 +  * Determine start of T10_PI or data payload iovec in ANY_LAYOUT
 +  * mode based upon data_direction.
 +  *
 +  * For DMA_TO_DEVICE, this is iov_out from copy_from_iter()
 +  * with the already recalculated iov_base + iov_len.

ITYM this is out_iter, which is already pointing to the right place

AFAICS, the actual use is correct, it's just that the comments are confused.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH-v2 01/11] lib/iovec: Add memcpy_fromiovec_out library function

2015-02-01 Thread Al Viro
On Mon, Feb 02, 2015 at 04:06:24AM +, Nicholas A. Bellinger wrote:
 From: Nicholas Bellinger n...@linux-iscsi.org
 
 This patch adds a new memcpy_fromiovec_out() library function which modifies
 the passed *iov following memcpy_fromiovec(), but also returns the next 
 current
 iovec pointer via **iov_out.
 
 This is useful for vhost ANY_LAYOUT support when guests are allowed to 
 generate
 incoming virtio request headers combined with subsequent SGL payloads into a
 single iovec.

Please, don't.  Just use copy_from_iter(); you are open-coding an uglier
variant of such.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH-v2 01/11] lib/iovec: Add memcpy_fromiovec_out library function

2015-02-01 Thread Al Viro
On Mon, Feb 02, 2015 at 04:44:12AM +, Al Viro wrote:
 On Mon, Feb 02, 2015 at 04:06:24AM +, Nicholas A. Bellinger wrote:
  From: Nicholas Bellinger n...@linux-iscsi.org
  
  This patch adds a new memcpy_fromiovec_out() library function which modifies
  the passed *iov following memcpy_fromiovec(), but also returns the next 
  current
  iovec pointer via **iov_out.
  
  This is useful for vhost ANY_LAYOUT support when guests are allowed to 
  generate
  incoming virtio request headers combined with subsequent SGL payloads into a
  single iovec.
 
 Please, don't.  Just use copy_from_iter(); you are open-coding an uglier
 variant of such.

PS: see vfs.git#for-davem (or postings on netdev with the same stuff).
I really hope to bury memcpy_...iovec...() crap for good; please, don't
reintroduce more of it.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[regression] fix 32-bit breakage in block device read(2) (was Re: 32-bit bug in iovec iterator changes)

2014-06-23 Thread Al Viro
On Sun, Jun 22, 2014 at 07:50:07AM -0400, Theodore Ts'o wrote:
 On Sun, Jun 22, 2014 at 02:00:32AM +0100, Al Viro wrote:
  
  PS: I agree that it's worth careful commenting, obviously, but
  before sending it to Linus (*with* comments) I want to get a
  confirmation that this one-liner actually fixes what Ted is seeing.
  I have reproduced it here, and that change makes the breakage go
  away in my testing, but I'd like to make sure that we are seeing the
  same thing.  Ted?
 
 Hep, that fixes things.  Thanks for explaining what was going on!

OK, here it is, hopefully with sufficient comments:

blkdev_read_iter() wants to cap the iov_iter by the amount of
data remaining to the end of device.  That's what iov_iter_truncate()
is for (trim iter-count if it's above the given limit).  So far,
so good, but the argument of iov_iter_truncate() is size_t, so on
32bit boxen (in case of a large device) we end up with that upper
limit truncated down to 32 bits *before* comparing it with iter-count.

Easily fixed by making iov_iter_truncate() take 64bit argument -
it does the right thing after such change (we only reach the
assignment in there when the current value of iter-count is greater
than the limit, i.e. for anything that would get truncated we don't
reach the assignment at all) and that argument is not the new
value of iter-count - it's an upper limit for such.

The overhead of passing u64 is not an issue - the thing is inlined,
so callers passing size_t won't pay any penalty.

Reported-by: Theodore Tso ty...@mit.edu
Tested-by: Theodore Tso ty...@mit.edu
Signed-off-by: Al Viro v...@zeniv.linux.org.uk
---

diff --git a/include/linux/uio.h b/include/linux/uio.h
index ddfdb53..17ae7e3 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -94,8 +94,20 @@ static inline size_t iov_iter_count(struct iov_iter *i)
return i-count;
 }
 
-static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
+/*
+ * Cap the iov_iter by given limit; note that the second argument is
+ * *not* the new size - it's upper limit for such.  Passing it a value
+ * greater than the amount of data in iov_iter is fine - it'll just do
+ * nothing in that case.
+ */
+static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
 {
+   /*
+* count doesn't have to fit in size_t - comparison extends both
+* operands to u64 here and any value that would be truncated by
+* conversion in assignement is by definition greater than all
+* values of size_t, including old i-count.
+*/
if (i-count  count)
i-count = count;
 }
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 32-bit bug in iovec iterator changes

2014-06-21 Thread Al Viro
On Sat, Jun 21, 2014 at 07:09:22PM -0400, Theodore Ts'o wrote:
 On Sat, Jun 21, 2014 at 06:53:07AM +0100, Al Viro wrote:
  
  ed include/linux/uio.h EOF
  /iov_iter_truncate/s/size_t/u64/
  w
  q
  EOF
  
  Could you check if that fixes the sucker?
 
 The following patch (attached at the end) appears to fix the problem,
 but looking at uio.h, I'm completely confused about *why* it fixes the
 problem.  In particular, iov_iter_iovec() makes no sense to me at all,
 and I don't understand how the calculation of iov_len makes any sense:
 
   .iov_len = min(iter-count,
  iter-iov-iov_len - iter-iov_offset),

Eh?   We have iov[0].iov_base..iov[0].iov_base+iov[0].iov_len - 1 for
area covered by the first iovec.  First iov_offset bytes have already
been consumed.  And at most count bytes matter.  So yes, this iov_len
will give you equivalent first iovec.

Said that, iov_iter_iovec() will die shortly - it's a rudiment of older
code, with almost no users left.  But yes, it is correct.

 It also looks like uio.h is mostly about offsets to memory pointers,
 and so why this would make a difference when the issue is the block
 device offset goes above 2**30?

It is, and your patch is a huge overkill.

 There must be deep magic going on here, and so I don't know if your
 s/size_t/u64/g substitation also extends to the various functions that
 have size_t in them:

No, it does not.  It's specifically about iov_iter_truncate(); moreover,
it matters to only one caller of that sucker.  Namely,

static ssize_t blkdev_read_iter(struct kiocb *iocb, struct iov_iter *to)
{
struct file *file = iocb-ki_filp;
struct inode *bd_inode = file-f_mapping-host;
loff_t size = i_size_read(bd_inode);
loff_t pos = iocb-ki_pos;

if (pos = size)
return 0;

size -= pos;
iov_iter_truncate(to, size);
return generic_file_read_iter(iocb, to);
}

What happens here is capping to-count, to guarantee that we won't even look
at anything past the end of block device.  Alternative fix would be to
have
if (pos = size)
return 0;
if (to-size + pos  size) {
/* note that size - pos fits into size_t in this case,
 * so it's OK to pass it to iov_iter_truncate().
 */
iov_iter_truncate(to, size - pos);
}
return generic_file_read_iter(iocb, to);
in there.  Other callers are passing it size_t values already, so we don't
need similar checks there.

Or we can make iov_iter_truncate() take an arbitrary u64 argument, seeing that
it's inlined anyway.  IMO it's more robust that way...

Anyway, does the following alone fix the problem you are seeing?

diff --git a/include/linux/uio.h b/include/linux/uio.h
index ddfdb53..dbb02d4 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -94,7 +94,7 @@ static inline size_t iov_iter_count(struct iov_iter *i)
return i-count;
 }
 
-static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
+static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
 {
if (i-count  count)
i-count = count;
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 32-bit bug in iovec iterator changes

2014-06-21 Thread Al Viro
On Sat, Jun 21, 2014 at 05:03:20PM -0700, James Bottomley wrote:

  Anyway, does the following alone fix the problem you are seeing?
  
  diff --git a/include/linux/uio.h b/include/linux/uio.h
  index ddfdb53..dbb02d4 100644
  --- a/include/linux/uio.h
  +++ b/include/linux/uio.h
  @@ -94,7 +94,7 @@ static inline size_t iov_iter_count(struct iov_iter *i)
  return i-count;
   }
   
  -static inline void iov_iter_truncate(struct iov_iter *i, size_t count)
  +static inline void iov_iter_truncate(struct iov_iter *i, u64 count)
   {
  if (i-count  count)
  i-count = count;
 
 Al, how can that work?  i-count is size_t, which is 32 bit, so we're
 going to get truncation errors.

No, we are not.  Look:
* comparison promotes both operands to u64 here, so its result is
accurate, no matter how large count is.  They are compared as natural
numbers.
* assignment converts count to size_t, which *would* truncate for
values that are greater than the maximal value representable by size_t.
But in that case it's by definition greater than i-count, so we do not
reach that assignment at all.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 32-bit bug in iovec iterator changes

2014-06-21 Thread Al Viro
On Sat, Jun 21, 2014 at 05:32:44PM -0700, James Bottomley wrote:
  No, we are not.  Look:
  * comparison promotes both operands to u64 here, so its result is
  accurate, no matter how large count is.  They are compared as natural
  numbers.
 
 True ... figured this out 10 seconds after sending the email.
 
  * assignment converts count to size_t, which *would* truncate for
  values that are greater than the maximal value representable by size_t.
  But in that case it's by definition greater than i-count, so we do not
  reach that assignment at all.
 
 OK, so what I still don't get is why isn't the compiler warning when we
 truncate a u64 to a u32?  We should get that warning in your new code,
 and we should have got that warning in fs/block_dev.c where it would
 have pinpointed the actual problem.

In which universe?

extern void f(unsigned int);

void g(unsigned long x)
{
f(x);
}

is perfectly valid C, with no warnings in sight.  f(1UL  32) might
give one, but not this...
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 32-bit bug in iovec iterator changes

2014-06-21 Thread Al Viro
On Sun, Jun 22, 2014 at 01:53:52AM +0100, Al Viro wrote:
 On Sat, Jun 21, 2014 at 05:32:44PM -0700, James Bottomley wrote:
   No, we are not.  Look:
 * comparison promotes both operands to u64 here, so its result is
   accurate, no matter how large count is.  They are compared as natural
   numbers.
  
  True ... figured this out 10 seconds after sending the email.
  
 * assignment converts count to size_t, which *would* truncate for
   values that are greater than the maximal value representable by size_t.
   But in that case it's by definition greater than i-count, so we do not
   reach that assignment at all.
  
  OK, so what I still don't get is why isn't the compiler warning when we
  truncate a u64 to a u32?  We should get that warning in your new code,
  and we should have got that warning in fs/block_dev.c where it would
  have pinpointed the actual problem.
 
 In which universe?
 
 extern void f(unsigned int);
 
 void g(unsigned long x)
 {
   f(x);
 }
 
 is perfectly valid C, with no warnings in sight.  f(1UL  32) might
 give one, but not this...

PS: I agree that it's worth careful commenting, obviously, but before sending
it to Linus (*with* comments) I want to get a confirmation that this one-liner
actually fixes what Ted is seeing.  I have reproduced it here, and that change
makes the breakage go away in my testing, but I'd like to make sure that we are
seeing the same thing.  Ted?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 32-bit bug in iovec iterator changes

2014-06-20 Thread Al Viro
On Fri, Jun 20, 2014 at 11:51:44PM -0400, Theodore Ts'o wrote:
 On Fri, Jun 20, 2014 at 08:38:20AM +1000, Dave Chinner wrote:
  
  Short reads are more likely a bug in all the iovec iterator stuff
  that got merged in from the vfs tree. ISTR a 32 bit-only bug in that
  stuff go past in to do with not being able to partition a 32GB block
  dev on a 32 bit system due to a 32 bit size_t overflow somewhere
 
 Dave Chinner called it.  
 
 Al, I'm seeing a regression which shows up using a 32-bit x86 kernel.
 The symptoms of the bug is when run under KVM, with a 5 GB /dev/vdc
 virtual block device, a read at offset 2 ** 30 fails with a short
 read:
 
 # dd if=/dev/vdc of=/dev/null bs=4k skip=262144 count=1
 0+0 records in
 0+0 records out
 0 bytes (0 B) copied, 0.0164144 s, 0.0 kB/s

Argh...

ed include/linux/uio.h EOF
/iov_iter_truncate/s/size_t/u64/
w
q
EOF

Could you check if that fixes the sucker?
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH -next 2/2] sun3_scsi: switch to -show_info()

2013-05-04 Thread Al Viro
On Fri, May 03, 2013 at 10:03:43PM +0200, Geert Uytterhoeven wrote:
 Ping?
 
 Now the build failure is also in Linus' tree:
 
 http://kisskb.ellerman.id.au/kisskb/buildresult/8674437/
 
 BTW, this patch depends on [PATCH 1/2] sun3_scsi: Fill in missing
 scsi_host_template.proc_info method

Collapsed together and applied, will go out today
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: question on sd_open()

2008-01-10 Thread Al Viro
On Wed, Jan 09, 2008 at 09:55:15PM +0100, Oliver Neukum wrote:
 Hello,
 
 is there a way to distinguish calls to sd_open() from mount() from
 calls to open() ?

No.  And there's no way to tell opening external log from either.  Or
from swapon.  Why the hell would the driver care?
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


trivial annotations in arcmsr

2007-10-28 Thread Al Viro

driver still has serious portability problems

Signed-off-by: Al Viro [EMAIL PROTECTED]
---
 drivers/scsi/arcmsr/arcmsr.h  |   32 ---
 drivers/scsi/arcmsr/arcmsr_attr.c |6 +-
 drivers/scsi/arcmsr/arcmsr_hba.c  |  170 ++---
 3 files changed, 103 insertions(+), 105 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
index ace7a15..3c38cd8 100644
--- a/drivers/scsi/arcmsr/arcmsr.h
+++ b/drivers/scsi/arcmsr/arcmsr.h
@@ -141,14 +141,14 @@ struct CMD_MESSAGE_FIELD
 #define IS_SG64_ADDR0x0100 /* bit24 */
 struct  SG32ENTRY
 {
-   uint32_tlength;
-   uint32_taddress;
+   __le32  length;
+   __le32  address;
 };
 struct  SG64ENTRY
 {
-   uint32_tlength;
-   uint32_taddress;
-   uint32_taddresshigh;
+   __le32  length;
+   __le32  address;
+   __le32  addresshigh;
 };
 struct SGENTRY_UNION
 {
@@ -339,13 +339,13 @@ struct MessageUnit_B
uint32_tdone_qbuffer[ARCMSR_MAX_HBB_POSTQUEUE];
uint32_tpostq_index;
uint32_tdoneq_index;
-   uint32_t*drv2iop_doorbell_reg;
-   uint32_t*drv2iop_doorbell_mask_reg;
-   uint32_t*iop2drv_doorbell_reg;
-   uint32_t*iop2drv_doorbell_mask_reg;
-   uint32_t*msgcode_rwbuffer_reg;
-   uint32_t*ioctl_wbuffer_reg;
-   uint32_t*ioctl_rbuffer_reg;
+   uint32_t__iomem *drv2iop_doorbell_reg;
+   uint32_t__iomem *drv2iop_doorbell_mask_reg;
+   uint32_t__iomem *iop2drv_doorbell_reg;
+   uint32_t__iomem *iop2drv_doorbell_mask_reg;
+   uint32_t__iomem *msgcode_rwbuffer_reg;
+   uint32_t__iomem *ioctl_wbuffer_reg;
+   uint32_t__iomem *ioctl_rbuffer_reg;
 };
 
 struct MessageUnit
@@ -374,7 +374,11 @@ struct AdapterControlBlock
/* Offset is used in making arc cdb physical to virtual calculations */
uint32_toutbound_int_enable;
 
-   struct MessageUnit *pmu;
+   union {
+   struct MessageUnit *pmu;
+   struct MessageUnit_A __iomem *  pmuA;
+   struct MessageUnit_B *  pmuB;
+   };
/* message unit ATU inbound base address0 */
 
uint32_tacb_flags;
@@ -558,7 +562,7 @@ struct SENSE_DATA
 
 extern void arcmsr_post_ioctldata2iop(struct AdapterControlBlock *);
 extern void arcmsr_iop_message_read(struct AdapterControlBlock *);
-extern struct QBUFFER *arcmsr_get_iop_rqbuffer(struct AdapterControlBlock *);
+extern struct QBUFFER __iomem *arcmsr_get_iop_rqbuffer(struct 
AdapterControlBlock *);
 extern struct class_device_attribute *arcmsr_host_attrs[];
 extern int arcmsr_alloc_sysfs_attr(struct AdapterControlBlock *);
 void arcmsr_free_sysfs_attr(struct AdapterControlBlock *acb);
diff --git a/drivers/scsi/arcmsr/arcmsr_attr.c 
b/drivers/scsi/arcmsr/arcmsr_attr.c
index d04d1aa..7d7b0a5 100644
--- a/drivers/scsi/arcmsr/arcmsr_attr.c
+++ b/drivers/scsi/arcmsr/arcmsr_attr.c
@@ -85,13 +85,13 @@ static ssize_t arcmsr_sysfs_iop_message_read(struct kobject 
*kobj,
allxfer_len++;
}
if (acb-acb_flags  ACB_F_IOPDATA_OVERFLOW) {
-   struct QBUFFER *prbuffer;
-   uint8_t *iop_data;
+   struct QBUFFER __iomem *prbuffer;
+   uint8_t __iomem *iop_data;
int32_t iop_len;
 
acb-acb_flags = ~ACB_F_IOPDATA_OVERFLOW;
prbuffer = arcmsr_get_iop_rqbuffer(acb);
-   iop_data = (uint8_t *)prbuffer-data;
+   iop_data = prbuffer-data;
iop_len = readl(prbuffer-data_len);
while (iop_len  0) {
acb-rqbuffer[acb-rqbuf_lastindex] = readb(iop_data);
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index f7a2528..aaee028 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -236,8 +236,8 @@ static int arcmsr_alloc_ccb_pool(struct AdapterControlBlock 
*acb)
uint32_t intmask_org;
int i, j;
 
-   acb-pmu = ioremap(pci_resource_start(pdev, 0), 
pci_resource_len(pdev, 0));
-   if (!acb-pmu) {
+   acb-pmuA = ioremap(pci_resource_start(pdev, 0), 
pci_resource_len(pdev, 0));
+   if (!acb-pmuA) {
printk(KERN_NOTICE arcmsr%d: memory mapping region 
fail \n,
acb-host

deal with resource allocation bugs in arcmsr

2007-10-28 Thread Al Viro

a) for type B we should _not_ iounmap() acb-pmu; it's not ioremapped.
b) for type B we should iounmap() two regions we _do_ ioremap.
c) if ioremap() fails, we need to bail out (and clean up).

Signed-off-by: Al Viro [EMAIL PROTECTED]
---
 drivers/scsi/arcmsr/arcmsr.h |9 -
 drivers/scsi/arcmsr/arcmsr_hba.c |   33 ++---
 2 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
index 3c38cd8..a67e29f 100644
--- a/drivers/scsi/arcmsr/arcmsr.h
+++ b/drivers/scsi/arcmsr/arcmsr.h
@@ -348,14 +348,6 @@ struct MessageUnit_B
uint32_t__iomem *ioctl_rbuffer_reg;
 };
 
-struct MessageUnit
-{
-   union
-   {
-   struct MessageUnit_Apmu_A;
-   struct MessageUnit_Bpmu_B;
-   } u;
-};
 /*
 ***
 ** Adapter Control Block
@@ -375,7 +367,6 @@ struct AdapterControlBlock
uint32_toutbound_int_enable;
 
union {
-   struct MessageUnit *pmu;
struct MessageUnit_A __iomem *  pmuA;
struct MessageUnit_B *  pmuB;
};
diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index aaee028..152af46 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -240,14 +240,18 @@ static int arcmsr_alloc_ccb_pool(struct 
AdapterControlBlock *acb)
if (!acb-pmuA) {
printk(KERN_NOTICE arcmsr%d: memory mapping region 
fail \n,
acb-host-host_no);
+   return -ENOMEM;
}
 
dma_coherent = dma_alloc_coherent(pdev-dev,
ARCMSR_MAX_FREECCB_NUM *
sizeof (struct CommandControlBlock) + 0x20,
dma_coherent_handle, GFP_KERNEL);
-   if (!dma_coherent)
+
+   if (!dma_coherent) {
+   iounmap(acb-pmuA);
return -ENOMEM;
+   }
 
acb-dma_coherent = dma_coherent;
acb-dma_coherent_handle = dma_coherent_handle;
@@ -331,8 +335,16 @@ static int arcmsr_alloc_ccb_pool(struct 
AdapterControlBlock *acb)
acb-pmuB = reg;
mem_base0 = ioremap(pci_resource_start(pdev, 0),
pci_resource_len(pdev, 0));
+   if (!mem_base0)
+   goto out;
+
mem_base1 = ioremap(pci_resource_start(pdev, 2),
pci_resource_len(pdev, 2));
+   if (!mem_base1) {
+   iounmap(mem_base0);
+   goto out;
+   }
+
reg-drv2iop_doorbell_reg = mem_base0 + ARCMSR_DRV2IOP_DOORBELL;
reg-drv2iop_doorbell_mask_reg = mem_base0 +
ARCMSR_DRV2IOP_DOORBELL_MASK;
@@ -357,6 +369,12 @@ static int arcmsr_alloc_ccb_pool(struct 
AdapterControlBlock *acb)
break;
}
return 0;
+
+out:
+   dma_free_coherent(acb-pdev-dev,
+   ARCMSR_MAX_FREECCB_NUM * sizeof(struct CommandControlBlock) + 
0x20,
+   acb-dma_coherent, acb-dma_coherent_handle);
+   return -ENOMEM;
 }
 
 static int arcmsr_probe(struct pci_dev *pdev,
@@ -449,7 +467,6 @@ static int arcmsr_probe(struct pci_dev *pdev,
free_irq(pdev-irq, acb);
  out_free_ccb_pool:
arcmsr_free_ccb_pool(acb);
-   iounmap(acb-pmu);
  out_release_regions:
pci_release_regions(pdev);
  out_host_put:
@@ -810,7 +827,6 @@ static void arcmsr_remove(struct pci_dev *pdev)
}
 
free_irq(pdev-irq, acb);
-   iounmap(acb-pmu);
arcmsr_free_ccb_pool(acb);
pci_release_regions(pdev);
 
@@ -1018,6 +1034,17 @@ static void arcmsr_stop_adapter_bgrb(struct 
AdapterControlBlock *acb)
 
 static void arcmsr_free_ccb_pool(struct AdapterControlBlock *acb)
 {
+   switch (acb-adapter_type) {
+   case ACB_ADAPTER_TYPE_A: {
+   iounmap(acb-pmuA);
+   break;
+   }
+   case ACB_ADAPTER_TYPE_B: {
+   struct MessageUnit_B *reg = acb-pmuB;
+   iounmap(reg-drv2iop_doorbell_reg - ARCMSR_DRV2IOP_DOORBELL);
+   iounmap(reg-ioctl_wbuffer_reg - ARCMSR_IOCTL_WBUFFER);
+   }
+   }
dma_free_coherent(acb-pdev-dev,
ARCMSR_MAX_FREECCB_NUM * sizeof (struct CommandControlBlock) + 
0x20,
acb-dma_coherent,
-- 
1.5.3.GIT


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


arcmsr: endianness bug

2007-10-28 Thread Al Viro

initializing a field in data shared with the card with
cpu_to_le32(something) | 0x10 is broken - the field
is, indeed, little-endian and we need cpu_to_le32() on
both parts.

Signed-off-by: Al Viro [EMAIL PROTECTED]
---
 drivers/scsi/arcmsr/arcmsr_hba.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 152af46..acbc50f 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -932,7 +932,7 @@ static void arcmsr_build_ccb(struct AdapterControlBlock 
*acb,
 
pdma_sg-addresshigh = address_hi;
pdma_sg-address = address_lo;
-   pdma_sg-length = length|IS_SG64_ADDR;
+   pdma_sg-length = 
length|cpu_to_le32(IS_SG64_ADDR);
psge += sizeof (struct SG64ENTRY);
arccdbsize += sizeof (struct SG64ENTRY);
}
-- 
1.5.3.GIT


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


fix reentrancy bug in arcmsr_get_iop_{r,w}qbuffer()

2007-10-28 Thread Al Viro

doh...

Signed-off-by: Al Viro [EMAIL PROTECTED]
---
 drivers/scsi/arcmsr/arcmsr_hba.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index acbc50f..d466a2d 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -1095,7 +1095,7 @@ static void arcmsr_iop_message_wrote(struct 
AdapterControlBlock *acb)
 
 struct QBUFFER __iomem *arcmsr_get_iop_rqbuffer(struct AdapterControlBlock 
*acb)
 {
-   static struct QBUFFER __iomem *qbuffer;
+   struct QBUFFER __iomem *qbuffer = NULL;
 
switch (acb-adapter_type) {
 
@@ -1116,7 +1116,7 @@ struct QBUFFER __iomem *arcmsr_get_iop_rqbuffer(struct 
AdapterControlBlock *acb)
 
 static struct QBUFFER __iomem *arcmsr_get_iop_wqbuffer(struct 
AdapterControlBlock *acb)
 {
-   static struct QBUFFER __iomem *pqbuffer;
+   struct QBUFFER __iomem *pqbuffer = NULL;
 
switch (acb-adapter_type) {
 
-- 
1.5.3.GIT

-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: /dev/sda with 8 byte offset

2007-09-27 Thread Al Viro
On Thu, Sep 27, 2007 at 08:45:09AM +0200, Stefan Richter wrote:
 Al Viro wrote:
  On Wed, Sep 26, 2007 at 10:55:11PM +0200, Stefan Richter wrote:
  Strange.  I haven't heard of this before.  From which vendor and model
  is the device, and do you know which chip is on its IDE bridge board?
  
  I've seen it, all right.  8 bytes stuck in FIFO, pl3507 IDE bridge,
  and judging by google search that turd is b0rken regardless of the
  OS (along the lines of works under Windows if you power-cycle it
  once in about half an hour).
  
  Suggested fix: use as a barf-bag; the authors of that thing certainly had
  done that.
 
 Sounds plausible.  I wasn't aware of the particular 8-byte-garbage
 symptom (or heard of it and forgot it).

IIRC, it can be triggered by the second INQUIRY during the same firewire
session (i.e. device survives only one INQUIRY after login and using e.g.
scsiinfo -i afterwards triggers that behaviour).

Can't verify that, since the hardware in question had been met its end
(after a week spent debugging that mess).  It was of the can't upgrade
the firmware variety and frankly, it's easier to spend $20 on a working
enclosure than to waste time on that dreck.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: /dev/sda with 8 byte offset

2007-09-26 Thread Al Viro
On Wed, Sep 26, 2007 at 10:55:11PM +0200, Stefan Richter wrote:
 Strange.  I haven't heard of this before.  From which vendor and model
 is the device, and do you know which chip is on its IDE bridge board?

I've seen it, all right.  8 bytes stuck in FIFO, pl3507 IDE bridge,
and judging by google search that turd is b0rken regardless of the
OS (along the lines of works under Windows if you power-cycle it
once in about half an hour).

Suggested fix: use as a barf-bag; the authors of that thing certainly had
done that.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Re: cciss: warning: right shift count = width of type

2007-08-12 Thread Al Viro
On Sun, Aug 12, 2007 at 03:21:57AM +0200, Rene Herman wrote:
 @@ -2609,13 +2609,13 @@ static void do_cciss_request(request_queue_t *q)
   } else {
   c-Request.CDBLen = 16;
   c-Request.CDB[1]= 0;
 - c-Request.CDB[2]= (start_blk  56)  0xff;//MSB
 - c-Request.CDB[3]= (start_blk  48)  0xff;
 - c-Request.CDB[4]= (start_blk  40)  0xff;
 - c-Request.CDB[5]= (start_blk  32)  0xff;
 - c-Request.CDB[6]= (start_blk  24)  0xff;
 - c-Request.CDB[7]= (start_blk  16)  0xff;
 - c-Request.CDB[8]= (start_blk   8)  0xff;
 + c-Request.CDB[2]= ((u64)start_blk  56)  0xff;   
 //MSB
 + c-Request.CDB[3]= ((u64)start_blk  48)  0xff;
 + c-Request.CDB[4]= ((u64)start_blk  40)  0xff;
 + c-Request.CDB[5]= ((u64)start_blk  32)  0xff;
 + c-Request.CDB[6]= ((u64)start_blk  24)  0xff;
 + c-Request.CDB[7]= ((u64)start_blk  16)  0xff;
 + c-Request.CDB[8]= ((u64)start_blk   8)  0xff;

put_unaligned(cpu_to_be64(start_blk), c-Request.CDB[2]);

which is what's happening here anyway.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] scsi: wd33c93 needs asm/irq.h

2007-07-22 Thread Al Viro
On Fri, Jul 20, 2007 at 11:50:50AM -0600, Matthew Wilcox wrote:
 On Fri, Jul 20, 2007 at 09:43:47PM +0400, Sergei Shtylyov wrote:
  Hello Christoph:
  
  +#include asm/irq.h
  
  These days that should probably be linux/irq.h.
  
  Not at all, linux/irq.h is something entirely different.
  
 Actually, linux/interrupt.h
 
 Not for enable/disable_irq.  For request_irq, yes.
 
 This is something that should be fixed.

Now it is...  FWIW, I suspect that absolute majority of asm/irq.h
uses can be removed now.

Next steps in irq.h/interrupt.h cleanups:
* scouring asm/irq.h like it had been done for sparc32;
the parts that are only used by relevant arch/ code should be
taken there and includes _in_ asm/irq.h trimmed to minimum
* separating tasklet.h, with interrupt.h still including it.
Using it where needed.
* asm/softirq.h (with stuff mostly taken there from asm/hardirq.h)
and linux/softirq.h; again interrupt.h still should include it.
* mechanical adding include of linux/interrupt.h to files that use
request_irq/free_irq/enable_irq/disable_irq/irqreturn_t/IRQF_...
--- in the next merge window:
* replace include of linux/interrupt.h in netdevice.h with
that of linux/softirq.h.
* trim uses of linux/interrupt.h that are not needed anymore.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] buggered kmalloc()

2007-07-20 Thread Al Viro
Signed-off-by: Al Viro [EMAIL PROTECTED]
---
diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index aebcd5f..7829ab1 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -1885,7 +1885,7 @@ static int iscsi_tcp_get_addr(struct iscsi_conn *conn, 
struct socket *sock,
struct sockaddr_in *sin;
int rc = 0, len;
 
-   addr = kmalloc(GFP_KERNEL, sizeof(*addr));
+   addr = kmalloc(sizeof(*addr), GFP_KERNEL);
if (!addr)
return -ENOMEM;
 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 0/3] clean gendisk out of scsi ULD structs

2007-07-05 Thread Al Viro
On Thu, Jul 05, 2007 at 02:06:36PM -0700, Kristen Carlson Accardi wrote:
 Since gendisk will now become part of struct scsi_device, we don't need
 to store this value in any private data structs where they already store
 scsi_device.  This series cleans up a few drivers which did this.

What the hell?  gendisks are *NOT* supposed to be embedded into other
data structures, you'll screw up the lifetime rules for them.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] sym53c500_cs: remove bogus call fo free_dma()

2007-01-30 Thread Al Viro

What DMA for 16bit pcmcia card, anyway?  We never do request_dma()
there and -dma_channel never changes since initialization to -1.
IOW, that call is dead code.

Signed-off-by: Al Viro [EMAIL PROTECTED]
---
 drivers/scsi/pcmcia/sym53c500_cs.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/pcmcia/sym53c500_cs.c 
b/drivers/scsi/pcmcia/sym53c500_cs.c
index 9fb0ea5..5b458d2 100644
--- a/drivers/scsi/pcmcia/sym53c500_cs.c
+++ b/drivers/scsi/pcmcia/sym53c500_cs.c
@@ -545,8 +545,6 @@ SYM53C500_release(struct pcmcia_device *link)
*/
if (shost-irq)
free_irq(shost-irq, shost);
-   if (shost-dma_channel != 0xff)
-   free_dma(shost-dma_channel);
if (shost-io_port  shost-n_io_port)
release_region(shost-io_port, shost-n_io_port);
 
-- 
1.5.0-rc2.GIT


-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] aacraid 2.6: Address sparse warnings

2005-04-08 Thread Al Viro
On Fri, Apr 08, 2005 at 02:41:58PM -0400, Salyzyn, Mark wrote:
 As long as this is guaranteed on all platforms to do the right thing ...
 paranoid about compiler optimizations. MarkH, this should be an easy
 regroup :-)

Well, let's see.

cpu_to_le32(~0U) - __cpu_to_le32(~0U) -
[on little-endian]
((__force __le32)(__u32)(~0U)) - ((__le32)(__u32)(~0U))
[on big-endian]
((__force __le32)__swab32((~0U))) -
((__le32)__swab32((~0U))) -
((__le32) (__builtin_constant_p((__u32)((~0U))) ?
 ___swab32(((~0U)))
: __fswab32(((~0U) -
((__le32) (__builtin_constant_p((__u32)((~0U))) ?
({
__u32 __x = ((~0U));
((__u32)(
(((__u32)(__x)  (__u32)0x00ffUL)  24) |
(((__u32)(__x)  (__u32)0xff00UL)   8) |
(((__u32)(__x)  (__u32)0x00ffUL)   8) |
(((__u32)(__x)  (__u32)0xff00UL)  24) ));
})
: __fswab32(((~0U)

And that's enough - __builtin_constant_p((__u32)((~0U)) is known at compile
time and is non-zero.  The rest is obvious and yes, gcc does understand
that __x is constant and so is the expression above.


IOW, all cpu_to_...(constant) will be simplified to constants at compile
time.  They will not be _constant_ _expressions_ from C point of view (IOW,
you can't say e.g.
enum {
A = cpu_to_le32(~0U)
};
- that's what __constant_cpu_to_le32() is for), but they certainly will be
evaluated by compiler.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] megaraid_mbox fix

2005-02-11 Thread Al Viro
wrong order of arguments in memset().

That, BTW, shows why cross-builds are useful - the only indication of
problem had been a new warning showing up in sparse output on alpha build
(number exceeding 256 got truncated).

Signed-off-by: Al Viro [EMAIL PROTECTED]

diff -urN RC11-rc3-bk8-base/drivers/scsi/megaraid/megaraid_mbox.c 
current/drivers/scsi/megaraid/megaraid_mbox.c
--- RC11-rc3-bk8-base/drivers/scsi/megaraid/megaraid_mbox.c 2005-02-11 
15:30:27.0 -0500
+++ current/drivers/scsi/megaraid/megaraid_mbox.c   2005-02-11 
20:54:12.434724497 -0500
@@ -4100,9 +4100,9 @@
mbox64  = raid_dev-sysfs_mbox64;
ldmap   = raid_dev-sysfs_buffer;
 
-   memset(uioc, sizeof(uioc_t), 0);
-   memset(mbox64, sizeof(mbox64_t), 0);
-   memset(ldmap, sizeof(raid_dev-curr_ldmap), 0);
+   memset(uioc, 0, sizeof(uioc_t));
+   memset(mbox64, 0, sizeof(mbox64_t));
+   memset(ldmap, 0, sizeof(raid_dev-curr_ldmap));
 
mbox= mbox64-mbox32;
raw_mbox= (char *)mbox;
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] fix warnings in new compat code for scsi

2005-02-03 Thread Al Viro
On Thu, Feb 03, 2005 at 08:39:22PM -0600, James Bottomley wrote:
 I just got around to applying and testing this.  I needed the attached
 to get around the compile warnings it gave me on ia64
 
 I've got to say, it doesn't look pretty to have the block layer
 compat_ioctl returning long but the scsi one returning int; likewise
 with the void __user *arg vs unsigned long arg.

Seconded.  Use of long is utter idiocy:
a) ioctl(2) is declared as int in userland headers
b) -ioctl() returns int
c) -compat_ioctl() is supposed to emulate -ioctl() of 32bit
platform, for fsck sake!  Even if none of the above would apply, it
would _still_ be 32bit.

And the first two reasons apply to -unlocked_ioctl() as well.  Could we
please undo that bright decision until it's too late?  I can do patches
for that in by tomorrow morning...
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] (7/27) *really* dumb typo in aacraid (cast to pointer to structure that doesn't exist ;-)

2005-02-02 Thread Al Viro
spot the typo...  It's harmless, in a sense that code compiles right,
but...
Signed-off-by: Al Viro [EMAIL PROTECTED]

diff -urN RC11-rc2-bk10-rme9652/drivers/scsi/aacraid/aachba.c 
RC11-rc2-bk10-aacraid/drivers/scsi/aacraid/aachba.c
--- RC11-rc2-bk10-rme9652/drivers/scsi/aacraid/aachba.c Tue Feb  1 21:44:42 2005
+++ RC11-rc2-bk10-aacraid/drivers/scsi/aacraid/aachba.c Wed Feb  2 08:12:13 2005
@@ -1047,7 +1047,7 @@
COMMAND_COMPLETE  8 | SAM_STAT_GOOD;
else {
struct scsi_device *sdev = cmd-device;
-   struct aac_dev *dev = (struct aav_dev *)sdev-host-hostdata;
+   struct aac_dev *dev = (struct aac_dev *)sdev-host-hostdata;
u32 cid = ID_LUN_TO_CONTAINER(sdev-id, sdev-lun);
printk(KERN_WARNING 
 synchronize_callback: synchronize failed, status = %d\n,
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html