Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: accessing unset entries in VLEN variable causes crash #221

Closed
WardF opened this issue Feb 12, 2016 · 5 comments
Closed

Python: accessing unset entries in VLEN variable causes crash #221

WardF opened this issue Feb 12, 2016 · 5 comments

Comments

@WardF
Copy link
Member

WardF commented Feb 12, 2016

Investigating issue originally reported at netcdf4-python:

It appears (from the other thread) that this issue is inherited from the C library. This issue will track progress confirming this and, if confirmed (as I suspect it will be), fixing the issue. See other thread for full details.

Attached is the test.nc file which reportedly crashes ncdump.
test.nc.gz

@WardF
Copy link
Member Author

WardF commented Feb 16, 2016

Ok, I've duplicated the issue with a new test in the C library in nc_test4/tst_empty_vlen_unlim.c. Note that this test returns 0 right now, as the crash happens when the file is read by ncdump.

We can mitigate the issue by changing the dimension length to fixed from NC_UNLIMITED.

The crash itself is happening in nc4_get_vara(), in libsrc4/nc4hdf.c:1161. The specific crash is happening in the following block of code:

     /* Copy the fill value into the rest of the data buffer. */
      filldata = (char *)data + real_data_size;
      for (i = 0; i < fill_len; i++)
        {
          if (var->type_info->nc_type_class == NC_STRING)
            {
              if (*(char **)fillvalue)
                {
                  if (!(*(char **)filldata = strdup(*(char **)fillvalue)))
                    BAIL(NC_ENOMEM);
                }
              else
                *(char **)filldata = NULL;
            }
          else
            memcpy(filldata, fillvalue, file_type_size); // <--- Crash Here
          filldata = (char *)filldata + file_type_size;
        }
    }

A fix is still pending; I need to determine if the error is in how the file is written or if it's in how the file is being read.

@WardF
Copy link
Member Author

WardF commented Feb 16, 2016

CC'ing @DennisHeimbigner in case anything leaps out to a more experienced eye.

@WardF
Copy link
Member Author

WardF commented Feb 18, 2016

In the example above, fillvalue = 0x00000000. A sample test program (and Google) confirm that memcpy with a null pointer results in undefined behavior. Still tracking down where/why this is occurring so that it can be properly fixed. The fix hopefully will be as easy as adding a check for a NULL fillvalue pointer, but I want to ensure this isn't indicative of an upstream problem somewhere.

WardF added a commit that referenced this issue Feb 18, 2016
…her issues into ncdump. Broader validation pending.
@WardF
Copy link
Member Author

WardF commented Feb 18, 2016

Ok, there is/was a logic flaw when using an NC_UNLIMITED dimension with a VLEN data type. More testing is needed to ensure I haven't added a different bug, but I'm somewhat optimistic at this point (the other tests are passing). I need to:

  • Wire new tests into cmake and autotools.
  • Expand the test somewhat beyond the basic case.

Hopefully we should have the fix in the master branch fairly soon.

WardF added a commit that referenced this issue Feb 19, 2016
…can be checked easily with ncdump. Added the test and associated files to both cmake and autotools build systems.
WardF added a commit that referenced this issue Feb 19, 2016
@WardF
Copy link
Member Author

WardF commented Feb 19, 2016

Issue fixed unless I hear differently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant