This is the mail archive of the ecos-patches@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Flash bound checking fails if "upper (0xffffffff)" flash mapping


sebastien Couret wrote:
I have ran into a particular problem with the flash. It's about flash address bound checking.
This problem only happens if the two following conditions are met :


- Assert are activated (CYG_INFRA_DEBUG set)
- Upper flash address is set to 0xffffffff


What happens is that 'flash_hwr_init()' (which is implemented differently for each flash hardware) sets flash_info.end to zero (0xffffffff+1 !))

As a result , in io/flash/current/src/flashiodev.c , following check fails !
(flashend=0 and endpos>0 ), so flash access is always refused !

#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
    char *endpos = startpos + *len - 1;
    char *flashend = MIN( (char *)flash_info.end, dev->end);
    if ( startpos < dev->start )
        return -EINVAL;
    if ( endpos > flashend )
        return -EINVAL;
#endif

May be the better corrective will be to change the way of computing flash_info.end (by just substracting one) in each hardware flash driver.

But it's quicker to change directly flashiodev.c.
Here 's a patch :

 diff -a -w -r -u io/flash/current/src/v1.7/ io/flash/current/src/flashiodev.c
--- io/flash/current/src/v1.7/flashiodev.c      Thu Apr 22 11:31:55 2004
+++ io/flash/current/src/flashiodev.c   Mon Apr 19 18:47:35 2004
@@ -126,7 +126,7 @@

 #ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
     char *endpos = startpos + *len - 1;
-    char *flashend = MIN( (char *)flash_info.end, dev->end);
+    char *flashend = MIN( (char *)flash_info.end - 1, dev->end);
[etc. similar]

Hi Sebastian,

I believe this patch just needs an extra adjustment. If you have a flash device that ends at 0xffffffff and the flashiodev device also ends there, then dev->end will also be 0xffffffff+1 i.e. 0. So the "if ( endpos > flashend )" check will fail because flashend will have been set to 0. Easily fixed anyway.

By the way, your mailer corrupted the patch - it was not whitespace clean, and one line had been wrapped. In future it would be easier if you can perhaps include the patch as an attachment. Thanks!

Jifl

Index: ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/ChangeLog,v
retrieving revision 1.35
diff -u -5 -p -r1.35 ChangeLog
--- ChangeLog	1 Dec 2003 14:33:58 -0000	1.35
+++ ChangeLog	29 Apr 2004 06:37:03 -0000
@@ -1,5 +1,11 @@
+2004-04-29  Sebastien Couret  <sebastien.couret@elios-informatique.com>
+2004-04-29  Jonathan Larmour  <jifl@eCosCentric.com>
+
+	* src/flashiodev.c: When checking flash upper bound, allow for end
+	of flash at 0xffffffff.
+
 2003-11-27  David Woodhouse  <dwmw2@infradead.org>

 	* src/flashiodev.c: Enable set_config() and implement
 	CYG_IO_SET_CONFIG_FLASH_FIS_NAME.
 	
Index: src/flashiodev.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/flash/current/src/flashiodev.c,v
retrieving revision 1.7
diff -u -5 -p -r1.7 flashiodev.c
--- src/flashiodev.c	1 Dec 2003 14:33:59 -0000	1.7
+++ src/flashiodev.c	29 Apr 2004 06:37:04 -0000
@@ -124,11 +124,11 @@ flashiodev_bread( cyg_io_handle_t handle
         Cyg_ErrNo err = ENOERR;


#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time char *endpos = startpos + *len - 1; - char *flashend = MIN( (char *)flash_info.end, dev->end); + char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1); if ( startpos < dev->start ) return -EINVAL; if ( endpos > flashend ) return -EINVAL; #endif @@ -152,11 +152,11 @@ flashiodev_bwrite( cyg_io_handle_t handl void *erraddr; char *startpos = dev->start + pos;

#ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
char *endpos = startpos + *len - 1;
- char *flashend = MIN( (char *)flash_info.end, dev->end);
+ char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
if ( startpos < dev->start )
return -EINVAL;
if ( endpos > flashend )
return -EINVAL;
#endif
@@ -186,12 +186,12 @@ flashiodev_get_config( cyg_io_handle_t h
cyg_io_flash_getconfig_erase_t *e = (cyg_io_flash_getconfig_erase_t *)buf;
char *startpos = dev->start + e->offset;


 #ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
             char *endpos = startpos + e->len - 1;
-		    char *flashend = MIN( (char *)flash_info.end, dev->end);
-			if ( startpos < dev->start )
+            char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);
+            if ( startpos < dev->start )
                 return -EINVAL;
             if ( endpos > flashend )
                 return -EINVAL;
 #endif
             e->flasherr = flash_erase( startpos, e->len, e->err_address );
@@ -216,11 +216,11 @@ flashiodev_get_config( cyg_io_handle_t h
     {
         cyg_io_flash_getconfig_blocksize_t *b =
             (cyg_io_flash_getconfig_blocksize_t *)buf;
 #ifdef CYGPKG_INFRA_DEBUG // don't bother checking this all the time
        char *startpos = dev->start + b->offset;
-	    char *flashend = MIN( (char *)flash_info.end, dev->end);
+	    char *flashend = MIN( (char *)flash_info.end - 1, dev->end - 1);

         if ( startpos < dev->start )
             return -EINVAL;
         if ( startpos > flashend )
             return -EINVAL;
@@ -242,12 +242,14 @@ static Cyg_ErrNo
 flashiodev_set_config( cyg_io_handle_t handle,
                        cyg_uint32 key,
                        const void* buf,
                        cyg_uint32* len)
 {
+#ifdef CYGNUM_IO_FLASH_BLOCK_CFG_FIS_1
 	struct cyg_devtab_entry *tab = (struct cyg_devtab_entry *)handle;
 	struct flashiodev_priv_t *dev = (struct flashiodev_priv_t *)tab->priv;
+#endif

     switch (key) {
 #ifdef CYGNUM_IO_FLASH_BLOCK_CFG_FIS_1
     case CYG_IO_SET_CONFIG_FLASH_FIS_NAME:
     {




-- eCosCentric http://www.eCosCentric.com/ The eCos and RedBoot experts --["No sense being pessimistic, it wouldn't work anyway"]-- Opinions==mine


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]