[PATCH] Fix CID 1415704 Integer overflowed argument

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
Hi!

Review appreciated!

Thanks, Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

patch.txt (901 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Thursday, 27 July 2017 14:10:28 CEST Volker Lendecke via samba-technical
wrote:
> Hi!

Hi Volker,

before we push any patch to Samba we should fix it in uid_wrapper first. The
reason is that we want to avoid reverting patches when we update to a new
version of uid_wrapper in Samba. So fixing it upstream first is important.

Are you ok with the attached patchset? If yes I would push them to uid_wrapper
upstream and then submit your patch for the Samba tree to autobuild.


Cheers,


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

uwrap_int_overflow_arg.patch.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Thu, Jul 27, 2017 at 04:02:00PM +0200, Andreas Schneider wrote:
> before we push any patch to Samba we should fix it in uid_wrapper first. The
> reason is that we want to avoid reverting patches when we update to a new
> version of uid_wrapper in Samba. So fixing it upstream first is important.
>
> Are you ok with the attached patchset? If yes I would push them to uid_wrapper
> upstream and then submit your patch for the Samba tree to autobuild.

My problem is that it can take really, really long until enough
patches have piled up to justify going through the wrapper release
process and then going through another round of review to finally get
the stuff into Samba. Having to wait many weeks to get a bugfix into
Samba stretches my patience a lot.

Also, if we did the right thing and not push the wrappers in one huge
blob patch but in individual patches to keep the proper history also
in Samba, we would not have a problem. The patches pushed to Samba in
advance would just be eliminated by a rebase because they already
exist. So if we followed good engineering practices, there would not
be any conflict at all.

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Thursday, 27 July 2017 18:25:46 CEST Volker Lendecke wrote:

> On Thu, Jul 27, 2017 at 04:02:00PM +0200, Andreas Schneider wrote:
> > before we push any patch to Samba we should fix it in uid_wrapper first.
> > The reason is that we want to avoid reverting patches when we update to a
> > new version of uid_wrapper in Samba. So fixing it upstream first is
> > important.
> >
> > Are you ok with the attached patchset? If yes I would push them to
> > uid_wrapper upstream and then submit your patch for the Samba tree to
> > autobuild.
> My problem is that it can take really, really long until enough
> patches have piled up to justify going through the wrapper release
> process and then going through another round of review to finally get
> the stuff into Samba. Having to wait many weeks to get a bugfix into
> Samba stretches my patience a lot.

We are fine applying individual patches to Samba if it is urgent. The last
time I've asked if it can wait till I do a release. We never set it as a hard
rule and we fixed issues in Samba before without doing a release of a wrapper.

We can continue arguing here for the next weeks but this will not get the
patches in ...

> Also, if we did the right thing and not push the wrappers in one huge
> blob patch but in individual patches to keep the proper history also
> in Samba, we would not have a problem. The patches pushed to Samba in
> advance would just be eliminated by a rebase because they already
> exist. So if we followed good engineering practices, there would not
> be any conflict at all.

Good engineering practice would be to *NOT* have them in the Samba source
tree! But I think we disagree on that ...


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Thu, Jul 27, 2017 at 06:36:16PM +0200, Andreas Schneider wrote:
> Good engineering practice would be to *NOT* have them in the Samba source
> tree! But I think we disagree on that ...

No, we don't. Just remove them. If we did not have them in the Samba
tree, Coverity would not have found that issue. Problem solved.

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thursday, 27 July 2017 16:02:00 CEST Andreas Schneider via samba-technical
wrote:
> On Thursday, 27 July 2017 14:10:28 CEST Volker Lendecke via samba-technical
>
> wrote:
> > Hi!
>
> Hi Volker,

> Are you ok with the attached patchset?

Are you fine with the mentioned patchset? Can I push it so we can move on and
fix it in uid_wrapper and samba source code?


Thanks,


        Andreas


--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Sat, Jul 29, 2017 at 11:29:14AM +0200, Andreas Schneider wrote:
> > Are you ok with the attached patchset?
>
> Are you fine with the mentioned patchset? Can I push it so we can move on and
> fix it in uid_wrapper and samba source code?

Yes, sure.

Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Sunday, 30 July 2017 17:22:01 CEST Volker Lendecke via samba-technical
wrote:
> On Sat, Jul 29, 2017 at 11:29:14AM +0200, Andreas Schneider wrote:
> > > Are you ok with the attached patchset?
> >
> > Are you fine with the mentioned patchset? Can I push it so we can move on
> > and fix it in uid_wrapper and samba source code?
>
> Yes, sure.

The attached patch adresses CID 1415704 corretly. It has already been applied
to the uid_wrapper repository and Coverity reported that the issue has been
eliminated.


Review and push appreciated.


Thanks,


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

0001-lib-Fix-integer-overflowed-argument-issue-with-strto.patch.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

AW: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
Hi,

i've looked through last commits on master and maybe found some mistake. Which function should be really used here?
Commit message: strtoul
Commit: strtol

I think strtol, because variable "n", that holds return value of function, is of type long.

Thanks

Andrej

-----Ursprüngliche Nachricht-----
Von: samba-technical [mailto:[hidden email]] Im Auftrag von Andreas Schneider via samba-technical
Gesendet: Donnerstag, 3. August 2017 10:56
An: [hidden email]; [hidden email]
Betreff: Re: [PATCH] Fix CID 1415704 Integer overflowed argument

On Sunday, 30 July 2017 17:22:01 CEST Volker Lendecke via samba-technical
wrote:
> On Sat, Jul 29, 2017 at 11:29:14AM +0200, Andreas Schneider wrote:
> > > Are you ok with the attached patchset?
> >
> > Are you fine with the mentioned patchset? Can I push it so we can
> > move on and fix it in uid_wrapper and samba source code?
>
> Yes, sure.

The attached patch adresses CID 1415704 corretly. It has already been applied to the uid_wrapper repository and Coverity reported that the issue has been eliminated.


Review and push appreciated.


Thanks,


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] Fix CID 1415704 Integer overflowed argument

Samba - samba-technical mailing list
On Friday, 4 August 2017 14:59:45 CEST Andrej Gessel via samba-technical
wrote:
> Hi,
>
> i've looked through last commits on master and maybe found some mistake.
> Which function should be really used here?
 Commit message: strtoul
> Commit: strtol
>
> I think strtol, because variable "n", that holds return value of function,
> is of type long.


Yes, the commit message is wrong. strtol is correct.


Thanks for reviewing!


        Andreas

--
Andreas Schneider                   GPG-ID: CC014E3D
Samba Team                             [hidden email]
www.samba.org

Loading...