Messages in this thread | | | Date | Mon, 21 Oct 2013 09:03:46 -0200 | Subject | Re: [PATCH] 9p: unsigned/signed wrap in p9/unix modes. | From | Geyslan Gregório Bem <> |
| |
2013/10/20 Eric Van Hensbergen <ericvh@gmail.com>: > Please resubmit a clean patch which includes the check of sscanf for exactly > the correct number of arguments and handles errors properly in other cases. > That last bit may be a bit problematic since right now the only errors are > prints and we seem to be otherwise silently failing. Of course, looks like > nothing else is checking return values from that function for error. We > could set rdev to an ERR_PTR(-EIO), and then go through and do a check in > all the places which matter (looks like there are a few places where rdev > just gets discarded -- might even be a good idea to not parse rdev unless we > need to by passing NULL to p9mode2unixmode. > > All in all, its a corner case which is only likely with a broken server, but > the full clean up would seem to be: > a) switch to u32's > b) pass NULL when rdev just gets discarded and don't bother parsing when > it is > c) check the sscanf return validity > d) on error set ERR_PTR in rdev and check on return before proceeding > > That's a lot of cleanup, I'll add it to my work queue if you don't have time > to rework your patch. >
Eric, I would like to try with your guidance.
> For the other patches, anyone you didn't see a response from me on today is > being pulled into my for-next queue. Thanks for the cleanups. > > -eric
Thanks for accept them.
> > > > > On Mon, Oct 7, 2013 at 7:18 PM, Geyslan Gregório Bem <geyslan@gmail.com> > wrote: >> >> Joe, >> >> Nice, I'll wait their reply, there are other p9 patches that I have >> already sent and am awaiting Eric's. >> >> Thank you again. >> >> Geyslan Gregório Bem >> hackingbits.com >> >> >> 2013/10/7 Joe Perches <joe@perches.com>: >> > On Mon, 2013-10-07 at 21:09 -0300, Geyslan Gregório Bem wrote: >> >> Joe, >> >> >> >> Thank you for reply. >> >> >> >> What do you think about: >> >> >> >> strncpy(ext, stat->extension, sizeof(ext)); >> >> + if (sscanf(ext, "%c %u %u", &type, &major, &minor) < >> >> 3) { >> >> + p9_debug(P9_DEBUG_ERROR, >> >> + "It's necessary define type, major >> >> and minor values when using P9_DMDEVICE"); >> >> + return res; >> >> + } >> >> switch (type) { >> >> case 'c': >> >> res |= S_IFCHR; >> >> break; >> >> ... >> >> *rdev = MKDEV(major, minor); >> > >> > I think the plan 9 folk should figure out what's right. >> > >> > > > -- 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/
| |