Re: [PATCH] fs/befs/linuxvfs.c: need signed cast for variable 'block'

[Chen Gang]

On 11/01/2013 10:41 AM, Chen Gang wrote:
> On 11/01/2013 04:45 AM, Greg KH wrote:
>> On Thu, Oct 31, 2013 at 12:08:33PM -0700, Kees Cook wrote:
>>> On Thu, Oct 31, 2013 at 12:06 PM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>>>> On Thu, Oct 31, 2013 at 09:53:59AM -0700, Kees Cook wrote:
>>>>
>>>>> If block (type sector_t) is unsigned, we shouldn't cast it signed.
>>>>> This entire code path should be removed. What is BEFS's expected
>>>>> maximum block size? (Looks like even befs_blocknr_t is u64, so nothing
>>>>> seems trivially in danger of wrapping.) I would also note that all the
>>>>> format strings are wrong too (%ld instead of %lu).
>>>>
>>>> FWIW, this
>>>>         res = befs_fblock2brun(sb, ds, block, &run);
>>>>         if (res != BEFS_OK) {
>>>>                 befs_error(sb,
>>>>                            "<--- befs_get_block() for inode %lu, block "
>>>>                            "%ld ERROR", inode->i_ino, block);
>>>>                 return -EFBIG;
>>>>         }
>>>> also looks wrong - ioctl(..., FIBMAP, ...) shouldn't be able to spew
>>>> printks on a valid fs and hitting it with block number greater than
>>>> file length will, AFAICS, trigger that.
>>>>
>>>> I agree that this code needs fixing, but just making gcc STFU about the
>>>> comparison would only serve to hide the problem.  Anybody familiar with
>>>> befs or willing to learn it?
>>>
>>> Agreed. MAINTAINERS shows it as orphaned. Perhaps it should be moved
>>> into staging?
>>
>> Only if we want to delete the thing.  I'll be glad to take it there, and
>> remove it in 2 releases and then if anyone complains, we can add it back
>> easily.  Just let me know.
>>
> 
> Excuse me, I am not quite familiar with BEFS, I guess your meaning is:
> 
>   "if it is no further more discussion (e.g. within 1 week, no members reply), you will remove it (take it to "drivers/staging" sub-directory)".
> 

Oh, for me, it is not suitable to move a file system sub-directory to
"drivers/*/" sub-directory. And I can not find any sub-directory like
'staging' under "fs" sub-directory, either.

Do we have any sub-directory like "staging" in "fs" sub-directory? if
no, do we have to create it or have to use another ways instead of?


After reference the "drivers/staging" explanation (list below), we know
it is only for drivers (not fs).

menuconfig STAGING
        bool "Staging drivers"
        default n
        ---help---
          This option allows you to select a number of drivers that are
          not of the "normal" Linux kernel quality level.  These drivers
          are placed here in order to get a wider audience to make use of
          them.  Please note that these drivers are under heavy
          development, may or may not work, and may contain userspace
          interfaces that most likely will be changed in the near
          future.

          Using any of these drivers will taint your kernel which might
          affect support options from both the community, and various
          commercial support organizations.

          If you wish to work on these drivers, to help improve them, or
          to report problems you have with them, please see the
          driver_name.README file in the drivers/staging/ directory to
          see what needs to be worked on, and who to contact.

          If in doubt, say N here.


Thanks.

> If what I guess is correct, I support you (else, please let me know)
> 
> 
> Thanks.
> 

-- 
Chen Gang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/